opensaf-devel — OpenSAF Development and Reviews

Re: [devel] Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

[Bangalore R. R. <rak...@rb...>]

Hi @Thang Nguyen<mailto:tha...@en...> ,

whats opensaf_consensus_lock and opensaf_write_test opensaf  ?

Why is only opensaf_consensus_lock update always , i see in case of reboot opensaf_write_test opensaf is updated also .

One more would be when we set up this etcd on the active only active does Monitoring not the standby until its first switchover to Active then both nodes perform Monitoring on the etcd .

Regards,
Rakesh


________________________________
From: Thang Nguyen <tha...@en...>
Sent: Wednesday, July 17, 2024 13:48
To: Bangalore Ramesh, Rakesh <rak...@rb...>; ope...@li... <ope...@li...>
Subject: [EXTERNAL] RE: Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf


Hi Rakesk,

I assume your use case invalid.

I suggest to stop/disable etcd nodes and test again.

About the etcd, it is recommended having at least three etcd nodes.



B.R/Thang



From: Bangalore Ramesh, Rakesh <rak...@rb...>
Sent: Wednesday, July 17, 2024 12:27 PM
To: Thang Nguyen <tha...@en...>; ope...@li...
Subject: Re: Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf



CAUTION - EXTERNAL EMAIL

Hi @Thang Nguyen<mailto:tha...@en...> ,





Have attached the logs related to it .

Scenario -  I have recreated the scenario by deleting 2 etcd nodes out of 3, which is the quorum we are using .


The fmd.conf  file as below -

#########################################

# TARGET SYSTEM ARCHITECTURE

# Choices are:

#   ATCA (default)

#   HP_CCLASS

#   HP_PROLIANT

#########################################

export FM_TARGET_SYSTEM_ARCH="ATCA"



# Healthcheck keys

export FMS_HA_ENV_HEALTHCHECK_KEY="Default"



# Promote active timer

export FMS_PROMOTE_ACTIVE_TIMER=0



# To enable self fencing either comment the following line to get a default value of 10 seconds,

# or set an appropriate  timeout value, (unit is milliseconds).

export FMS_NODE_ISOLATION_TIMEOUT=0



# To enable remote fencing change to 1

#export FMS_USE_REMOTE_FENCING=0



# To enable split brain prevention, change to 1

export FMS_SPLIT_BRAIN_PREVENTION=1



# Used with split brain prevention, this controls

# the expiration time of takeover requests (unit is seconds)

export FMS_TAKEOVER_REQUEST_VALID_TIME=20



# Full path to key-value store plugin

export FMS_KEYVALUE_STORE_PLUGIN_CMD=/opt/opensaf/osaf-etcd3.plugin



# In the event of SCs being split into network partitions, we can try to make

# the active SC reside in the largest network partition. If it is preferable

# to keep the current SC active, then set this to 0

# Default is 1

#export FMS_TAKEOVER_PRIORITISE_PARTITION_SIZE=1



# If FMS_TAKEOVER_PRIORITISE_PARTITION_SIZE is set to 1, wait until

# this number of seconds for MDS events before making a decision

# on partition size. Default is 4 seconds

#export FMS_TAKEOVER_PRIORITISE_PARTITION_SIZE_MDS_WAIT_TIME=4



# Default behaviour is not to allow promotion of this node to Active

# unless a lock can be obtained, if split brain prevention is enabled.

# Uncomment the next line to allow promotion of this node at cluster startup,

# if a peer SC can be seen and we have a lower node ID, in the event the

# consensus service is not available.

# Also if the consensus service is down, but a peer SC can be seen,

# then an active SC may remain active.

# This mode should not be used together with the roaming SC feature

# Default is 0

export FMS_RELAXED_NODE_PROMOTION=1



# FM will supervise transitions to the ACTIVE role when this variable is set to

# a non-zero value. The value is the time in the unit of 10 ms to wait for a

# role change to ACTIVE to take effect. If AMF has not give FM an active

# assignment within this time, the node will be rebooted.

#export FMS_ACTIVATION_SUPERVISION_TIMER=30000



# Uncomment the next line to enable info level logging

#args="--loglevel=info"



# Uncomment the next line to enable trace

#args="--tracemask=0xffffffff"



# Only log priority LOG_WARNING and higher to the system log file.

# All logging will be recorded in a new node local log file $PKGLOGDIR/osaf.log.

# Uncomment the next line to enable this service to log to OpenSAF node local log file.

# export OSAF_LOCAL_NODE_LOG=1



# THREAD_TRACE_BUFFER variable enables the tracing, writes the trace

# to thread based buffer in circular fashion. The trace buffers will

# be flushed to file if an abnormal end hits, i.e. LOG_ER is called

# The value of THREAD_TRACE_BUFFER indicates the number of trace strings

# in a buffer. The length of a trace string is at most 256 characters.

# It can be disabled if set THREAD_TRACE_BUFFER as 0, the maximum value

# can be set as 65535.

# export THREAD_TRACE_BUFFER=10240


Regards,
Rakesh

________________________________

From: Thang Nguyen <tha...@en...<mailto:tha...@en...>>
Sent: Wednesday, July 17, 2024 08:29
To: Bangalore Ramesh, Rakesh <rak...@rb...<mailto:rak...@rb...>>; ope...@li...<mailto:ope...@li...> <ope...@li...<mailto:ope...@li...>>
Subject: [EXTERNAL] RE: Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf



Hi Rakesh,
>From my understanding, if export the below in fmd.conf
export FMS_RELAXED_NODE_PROMOTION=1

The SC will not be rebooted if it lost connection to etcd consensus service but still see the peer SC.
Could you share the fmd.conf and all the syslog ?

B.R/
Thang D Nguyen

-----Original Message-----
From: Bangalore Ramesh, Rakesh <rak...@rb...<mailto:rak...@rb...>>
Sent: Friday, July 12, 2024 1:49 PM
To: ope...@li...<mailto:ope...@li...>
Subject: [devel] Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

CAUTION - EXTERNAL EMAIL

Hi All,

I am trying to avoid opensaf bringing node down cause of no quorum with etcd cluster .

I saw this config in fmd. eventhough i have enabled it ,

# Default behaviour is not to allow promotion of this node to Active # unless a lock can be obtained, if split brain prevention is enabled.
# Uncomment the next line to allow promotion of this node at cluster startup, # if a peer SC can be seen and we have a lower node ID, in the event the # consensus service is not available.
# Also if the consensus service is down, but a peer SC can be seen, # then an active SC may remain active.
# This mode should not be used together with the roaming SC feature # Default is 0 #export FMS_RELAXED_NODE_PROMOTION=0

opensaf fails with error given below -


Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: NO (KeyValue::Execute): Executed '/opt/opensaf/osaf-etcd3.plugin unlock "SC-1"', returning 2 Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: WA Unlock failed (6) Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: ER Failed to demote this node from consensus service Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: WA Failed to retrieve search continuation, client died ?
Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: NO Implementer disconnected 4 <21, 2010f> (safAmfService) Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: NO Implementer (applier) connected: 13 (@safAmfService2010f) <21, 2010f>


Need help , how to use this config , so even if quorum is failed , I need the instance to not go from reboot .

Regards,
Rakesh

Disclaimer

This e-mail together with any attachments may contain information of Ribbon Communications Inc. and its Affiliates that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.

_______________________________________________
Opensaf-devel mailing list
Ope...@li...<mailto:Ope...@li...>
https://lists.sourceforge.net/lists/listinfo/opensaf-devel<https://lists.sourceforge.net/lists/listinfo/opensaf-devel>

The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection.

Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions.

Re: [devel] Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

[Bangalore R. R. <rak...@rb...>]

Hi

I tried stopping also , basically when quorum fails that's is 2 Outta 3 nodes are not reachable then node goes to reboot .


If etcdctl command fails when quorum fails .which is causing issue .

Regards
Rakesh
________________________________
From: Thang Nguyen <tha...@en...>
Sent: Wednesday, July 17, 2024 08:29
To: Bangalore Ramesh, Rakesh <rak...@rb...>; ope...@li... <ope...@li...>
Subject: [EXTERNAL] RE: Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

Hi Rakesh,
>From my understanding, if export the below in fmd.conf
export FMS_RELAXED_NODE_PROMOTION=1

The SC will not be rebooted if it lost connection to etcd consensus service but still see the peer SC.
Could you share the fmd.conf and all the syslog ?

B.R/
Thang D Nguyen

-----Original Message-----
From: Bangalore Ramesh, Rakesh <rak...@rb...>
Sent: Friday, July 12, 2024 1:49 PM
To: ope...@li...
Subject: [devel] Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

CAUTION - EXTERNAL EMAIL

Hi All,

I am trying to avoid opensaf bringing node down cause of no quorum with etcd cluster .

I saw this config in fmd. eventhough i have enabled it ,

# Default behaviour is not to allow promotion of this node to Active # unless a lock can be obtained, if split brain prevention is enabled.
# Uncomment the next line to allow promotion of this node at cluster startup, # if a peer SC can be seen and we have a lower node ID, in the event the # consensus service is not available.
# Also if the consensus service is down, but a peer SC can be seen, # then an active SC may remain active.
# This mode should not be used together with the roaming SC feature # Default is 0 #export FMS_RELAXED_NODE_PROMOTION=0

opensaf fails with error given below -


Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: NO (KeyValue::Execute): Executed '/opt/opensaf/osaf-etcd3.plugin unlock "SC-1"', returning 2 Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: WA Unlock failed (6) Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: ER Failed to demote this node from consensus service Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: WA Failed to retrieve search continuation, client died ?
Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: NO Implementer disconnected 4 <21, 2010f> (safAmfService) Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: NO Implementer (applier) connected: 13 (@safAmfService2010f) <21, 2010f>


Need help , how to use this config , so even if quorum is failed , I need the instance to not go from reboot .

Regards,
Rakesh

Disclaimer

This e-mail together with any attachments may contain information of Ribbon Communications Inc. and its Affiliates that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.

_______________________________________________
Opensaf-devel mailing list
Ope...@li...
https://lists.sourceforge.net/lists/listinfo/opensaf-devel<https://lists.sourceforge.net/lists/listinfo/opensaf-devel>

The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection.

Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions.

Re: [devel] Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

[Bangalore R. R. <rak...@rb...>]

Hi @Thang Nguyen<mailto:tha...@en...> ,


Have attached the logs related to it .

Scenario -  I have recreated the scenario by deleting 2 etcd nodes out of 3, which is the quorum we are using .


The fmd.conf  file as below -

#########################################
# TARGET SYSTEM ARCHITECTURE
# Choices are:
#   ATCA (default)
#   HP_CCLASS
#   HP_PROLIANT
#########################################
export FM_TARGET_SYSTEM_ARCH="ATCA"

# Healthcheck keys
export FMS_HA_ENV_HEALTHCHECK_KEY="Default"

# Promote active timer
export FMS_PROMOTE_ACTIVE_TIMER=0

# To enable self fencing either comment the following line to get a default value of 10 seconds,
# or set an appropriate  timeout value, (unit is milliseconds).
export FMS_NODE_ISOLATION_TIMEOUT=0

# To enable remote fencing change to 1
#export FMS_USE_REMOTE_FENCING=0

# To enable split brain prevention, change to 1
export FMS_SPLIT_BRAIN_PREVENTION=1

# Used with split brain prevention, this controls
# the expiration time of takeover requests (unit is seconds)
export FMS_TAKEOVER_REQUEST_VALID_TIME=20

# Full path to key-value store plugin
export FMS_KEYVALUE_STORE_PLUGIN_CMD=/opt/opensaf/osaf-etcd3.plugin

# In the event of SCs being split into network partitions, we can try to make
# the active SC reside in the largest network partition. If it is preferable
# to keep the current SC active, then set this to 0
# Default is 1
#export FMS_TAKEOVER_PRIORITISE_PARTITION_SIZE=1

# If FMS_TAKEOVER_PRIORITISE_PARTITION_SIZE is set to 1, wait until
# this number of seconds for MDS events before making a decision
# on partition size. Default is 4 seconds
#export FMS_TAKEOVER_PRIORITISE_PARTITION_SIZE_MDS_WAIT_TIME=4

# Default behaviour is not to allow promotion of this node to Active
# unless a lock can be obtained, if split brain prevention is enabled.
# Uncomment the next line to allow promotion of this node at cluster startup,
# if a peer SC can be seen and we have a lower node ID, in the event the
# consensus service is not available.
# Also if the consensus service is down, but a peer SC can be seen,
# then an active SC may remain active.
# This mode should not be used together with the roaming SC feature
# Default is 0
export FMS_RELAXED_NODE_PROMOTION=1

# FM will supervise transitions to the ACTIVE role when this variable is set to
# a non-zero value. The value is the time in the unit of 10 ms to wait for a
# role change to ACTIVE to take effect. If AMF has not give FM an active
# assignment within this time, the node will be rebooted.
#export FMS_ACTIVATION_SUPERVISION_TIMER=30000

# Uncomment the next line to enable info level logging
#args="--loglevel=info"

# Uncomment the next line to enable trace
#args="--tracemask=0xffffffff"

# Only log priority LOG_WARNING and higher to the system log file.
# All logging will be recorded in a new node local log file $PKGLOGDIR/osaf.log.
# Uncomment the next line to enable this service to log to OpenSAF node local log file.
# export OSAF_LOCAL_NODE_LOG=1

# THREAD_TRACE_BUFFER variable enables the tracing, writes the trace
# to thread based buffer in circular fashion. The trace buffers will
# be flushed to file if an abnormal end hits, i.e. LOG_ER is called
# The value of THREAD_TRACE_BUFFER indicates the number of trace strings
# in a buffer. The length of a trace string is at most 256 characters.
# It can be disabled if set THREAD_TRACE_BUFFER as 0, the maximum value
# can be set as 65535.
# export THREAD_TRACE_BUFFER=10240


Regards,
Rakesh
________________________________
From: Thang Nguyen <tha...@en...>
Sent: Wednesday, July 17, 2024 08:29
To: Bangalore Ramesh, Rakesh <rak...@rb...>; ope...@li... <ope...@li...>
Subject: [EXTERNAL] RE: Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

Hi Rakesh,
>From my understanding, if export the below in fmd.conf
export FMS_RELAXED_NODE_PROMOTION=1

The SC will not be rebooted if it lost connection to etcd consensus service but still see the peer SC.
Could you share the fmd.conf and all the syslog ?

B.R/
Thang D Nguyen

-----Original Message-----
From: Bangalore Ramesh, Rakesh <rak...@rb...>
Sent: Friday, July 12, 2024 1:49 PM
To: ope...@li...
Subject: [devel] Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

CAUTION - EXTERNAL EMAIL

Hi All,

I am trying to avoid opensaf bringing node down cause of no quorum with etcd cluster .

I saw this config in fmd. eventhough i have enabled it ,

# Default behaviour is not to allow promotion of this node to Active # unless a lock can be obtained, if split brain prevention is enabled.
# Uncomment the next line to allow promotion of this node at cluster startup, # if a peer SC can be seen and we have a lower node ID, in the event the # consensus service is not available.
# Also if the consensus service is down, but a peer SC can be seen, # then an active SC may remain active.
# This mode should not be used together with the roaming SC feature # Default is 0 #export FMS_RELAXED_NODE_PROMOTION=0

opensaf fails with error given below -


Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: NO (KeyValue::Execute): Executed '/opt/opensaf/osaf-etcd3.plugin unlock "SC-1"', returning 2 Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: WA Unlock failed (6) Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: ER Failed to demote this node from consensus service Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: WA Failed to retrieve search continuation, client died ?
Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: NO Implementer disconnected 4 <21, 2010f> (safAmfService) Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: NO Implementer (applier) connected: 13 (@safAmfService2010f) <21, 2010f>


Need help , how to use this config , so even if quorum is failed , I need the instance to not go from reboot .

Regards,
Rakesh

Disclaimer

This e-mail together with any attachments may contain information of Ribbon Communications Inc. and its Affiliates that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.

_______________________________________________
Opensaf-devel mailing list
Ope...@li...
https://lists.sourceforge.net/lists/listinfo/opensaf-devel<https://lists.sourceforge.net/lists/listinfo/opensaf-devel>

The information in this email is confidential and may be legally privileged. It is intended solely for the addressee. Any opinions expressed are mine and do not necessarily represent the opinions of the Company. Emails are susceptible to interference. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitted to be taken in reliance on it, is strictly prohibited and may be unlawful. If you have received this message in error, do not open any attachments but please notify the Endava Service Desk on (+44 (0)870 423 0187), and delete this message from your system. The sender accepts no responsibility for information, errors or omissions in this email, or for its use or misuse, or for any act committed or omitted in connection with this communication. If in doubt, please verify the authenticity of the contents with the sender. Please rely on your own virus checkers as no responsibility is taken by the sender for any damage rising out of any bug or virus infection.

Endava plc is a company registered in England under company number 5722669 whose registered office is at 125 Old Broad Street, London, EC2N 1AR, United Kingdom. Endava plc is the Endava group holding company and does not provide any services to clients. Each of Endava plc and its subsidiaries is a separate legal entity and has no liability for another such entity's acts or omissions.

[devel] Regarding FMS_RELAXED_NODE_PROMOTION in fmd.conf

[Bangalore R. R. <rak...@rb...>]

Hi  All,

I am trying to avoid opensaf bringing node down cause of  no quorum with etcd cluster .

I saw this config in fmd.  eventhough  i have enabled it ,

# Default behaviour is not to allow promotion of this node to Active
# unless a lock can be obtained, if split brain prevention is enabled.
# Uncomment the next line to allow promotion of this node at cluster startup,
# if a peer SC can be seen and we have a lower node ID, in the event the
# consensus service is not available.
# Also if the consensus service is down, but a peer SC can be seen,
# then an active SC may remain active.
# This mode should not be used together with the roaming SC feature
# Default is 0
#export FMS_RELAXED_NODE_PROMOTION=0

 opensaf fails with error given below -


Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: NO (KeyValue::Execute): Executed '/opt/opensaf/osaf-etcd3.plugin unlock "SC-1"', returning 2
Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: WA Unlock failed (6)
Jul 11 23:24:15 rakeshgr01 osafamfd[6898]: ER Failed to demote this node from consensus service
Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: WA Failed to retrieve search continuation, client died ?
Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: NO Implementer disconnected 4 <21, 2010f> (safAmfService)
Jul 11 23:24:15 rakeshgr01 osafimmnd[5987]: NO Implementer (applier) connected: 13 (@safAmfService2010f) <21, 2010f>


Need help , how to use this config , so even if quorum is failed , I need the instance to not go from reboot .

Regards,
Rakesh

Disclaimer

This e-mail together with any attachments may contain information of Ribbon Communications Inc. and its Affiliates that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.

Re: [devel] [PATCH 1/1] amf: handle saAmfCtDefClcCliTimeout correctly for non-proxied NPI component [#3355]

[Thien M. H. <thi...@de...>]

Hi Thang,

ACK with a comment.

Best Regards,
Thien

-----Original Message-----
From: Thang Duc Nguyen <tha...@de...> 
Sent: Thursday, June 27, 2024 7:27 PM
To: Thien Minh Huynh <thi...@de...>; Dat Tran Quoc Phan <dat...@de...>
Cc: ope...@li...; Thang Duc Nguyen <tha...@de...>
Subject: [PATCH 1/1] amf: handle saAmfCtDefClcCliTimeout correctly for non-proxied NPI component [#3355]

Non-proxied NPI component uses the terminate CLC CLI command when terminating. Any update to saAmfCtDefClcCliTimeout does not take effect.
The fix is to correct this issue.
---
 src/amf/amfnd/compdb.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc index ebe9d1f5b..7d6e532c5 100644
--- a/src/amf/amfnd/compdb.cc
+++ b/src/amf/amfnd/compdb.cc
@@ -1213,6 +1213,10 @@ static void init_clc_cli_attributes(AVND_COMP *comp,
       comp->term_cbk_timeout = cmd->timeout;
       comp->use_comptype_attr->set(TerminateCallbackTimeout);
       comp->use_comptype_attr->set(CompTerminateTimeout);

[Thien] m_AVND_COMP_TYPE_IS_PREINSTANTIABLE is redundant. Should be removed.
+    } else if(!m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(comp) &&
+              (!m_AVND_COMP_TYPE_IS_PROXIED(comp))) {
+      cmd->timeout = comptype->saAmfCtDefClcCliTimeout;
+      comp->use_comptype_attr->set(CompTerminateTimeout);
     } else
       cmd->timeout = comptype->saAmfCtDefClcCliTimeout;
   }
--
2.25.1

Re: [devel] [PATCH 1/1] smf: fix one step upgrade failed [#3354]

[Thien M. H. <thi...@de...>]

Hi Thang,

ACK from me.

Best Regards,
Thien

-----Original Message-----
From: Thang Duc Nguyen <tha...@de...> 
Sent: Tuesday, June 25, 2024 8:56 AM
To: Thien Minh Huynh <thi...@de...>; Dat Tran Quoc Phan <dat...@de...>
Cc: ope...@li...; Thang Duc Nguyen <tha...@de...>
Subject: [PATCH 1/1] smf: fix one step upgrade failed [#3354]

In large cluster or system under high load, during one step upgrade, SMF orders AMF to lock node group(NG). There are many request to IMM to update attribute and it causes the timeout respond from IMM to AMF. SMF receives timeout then retry lock again and again while the first lock still on going. When the first lock is successful and the request lock again from SMF will receive NO_OP error from AMF.

In this case, NO_OP should be considered as a success.
---
 src/smf/smfd/SmfAdminState.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc index 958b7ae82..c20df8d74 100755
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -926,6 +926,9 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
           saImmOmAdminOperationInvoke_2(ownerHandle_, &nodeGroupName, 0,
                                         adminOp, params, &oi_rc,
                                         smfd_cb->adminOpTimeout);
+      if ((imm_rc != SA_AIS_OK) || (oi_rc != SA_AIS_OK))
+        LOG_WA("%s: imm_rc: %s, oi_rc: %s", __FUNCTION__,
+            saf_error(imm_rc), saf_error(oi_rc));
       if ((imm_rc == SA_AIS_ERR_TRY_AGAIN) ||
           (imm_rc == SA_AIS_OK && oi_rc == SA_AIS_ERR_TRY_AGAIN)) {
         base::Sleep(base::MillisToTimespec(2000));
@@ -933,7 +936,8 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
       } else if (imm_rc == SA_AIS_ERR_TIMEOUT) {
         // Retry
         continue;
-      } else if (imm_rc == SA_AIS_ERR_NO_OP) {
+      } else if ((imm_rc == SA_AIS_ERR_NO_OP) ||
+                (oi_rc == SA_AIS_ERR_NO_OP)) {
         // If an admin operation is already performed SA_AIS_ERR_NO_OP
         // is returned. Treat this as OK, just log it and return
         // operation success
--
2.25.1

[devel] [PATCH 0/1] Review Request for amf: handle saAmfCtDefClcCliTimeout correctly for non-proxied NPI component [#3355]

[thang.d.nguyen <tha...@de...>]

Summary: amf: handle saAmfCtDefClcCliTimeout correctly for non-proxied NPI component [#3355]
Review request for Ticket(s): 3355
Peer Reviewer(s): *** LIST THE TECH REVIEWER(S) / MAINTAINER(S) HERE ***
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3355
Base revision: 1bb946c39860157532151ce5f33a2f0154e48ffe
Personal repository: git://git.code.sf.net/u/thangng/review

--------------------------------
Impacted area       Impact y/n
--------------------------------
 Docs                    n
 Build system            n
 RPM/packaging           n
 Configuration files     n
 Startup scripts         n
 SAF services            y
 OpenSAF services        n
 Core libraries          n
 Samples                 n
 Tests                   n
 Other                   n


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

revision cccecea9e8a2afc2ffa55c1c1b5c3516af95fd12
Author:	thang.d.nguyen <tha...@de...>
Date:	Thu, 27 Jun 2024 18:49:25 +0700

amf: handle saAmfCtDefClcCliTimeout correctly for non-proxied NPI component [#3355]

Non-proxied NPI component uses the terminate CLC CLI command
when terminating. Any update to saAmfCtDefClcCliTimeout does
not take effect.
The fix is to correct this issue.



Complete diffstat:
------------------
 src/amf/amfnd/compdb.cc | 4 ++++
 1 file changed, 4 insertions(+)


Testing Commands:
-----------------
N/A

Testing, Expected Results:
--------------------------
N/A

Conditions of Submission:
-------------------------
Ack from reviewer

Arch      Built     Started    Linux distro
-------------------------------------------
mips        n          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 ~/.gitconfig file (i.e. user.name, user.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.

[devel] [PATCH 1/1] amf: handle saAmfCtDefClcCliTimeout correctly for non-proxied NPI component [#3355]

[thang.d.nguyen <tha...@de...>]

Non-proxied NPI component uses the terminate CLC CLI command
when terminating. Any update to saAmfCtDefClcCliTimeout does
not take effect.
The fix is to correct this issue.
---
 src/amf/amfnd/compdb.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/amf/amfnd/compdb.cc b/src/amf/amfnd/compdb.cc
index ebe9d1f5b..7d6e532c5 100644
--- a/src/amf/amfnd/compdb.cc
+++ b/src/amf/amfnd/compdb.cc
@@ -1213,6 +1213,10 @@ static void init_clc_cli_attributes(AVND_COMP *comp,
       comp->term_cbk_timeout = cmd->timeout;
       comp->use_comptype_attr->set(TerminateCallbackTimeout);
       comp->use_comptype_attr->set(CompTerminateTimeout);
+    } else if(!m_AVND_COMP_TYPE_IS_PREINSTANTIABLE(comp) &&
+              (!m_AVND_COMP_TYPE_IS_PROXIED(comp))) {
+      cmd->timeout = comptype->saAmfCtDefClcCliTimeout;
+      comp->use_comptype_attr->set(CompTerminateTimeout);
     } else
       cmd->timeout = comptype->saAmfCtDefClcCliTimeout;
   }
-- 
2.25.1

[devel] [PATCH 0/1] Review Request for smf: fix one step upgrade failed [#3354]

[thang.d.nguyen <tha...@de...>]

Summary: smf: fix one step upgrade failed [#3354]
Review request for Ticket(s): 3354
Peer Reviewer(s): *** LIST THE TECH REVIEWER(S) / MAINTAINER(S) HERE ***
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3354
Base revision: 1bb946c39860157532151ce5f33a2f0154e48ffe
Personal repository: git://git.code.sf.net/u/thangng/review

--------------------------------
Impacted area       Impact y/n
--------------------------------
 Docs                    n
 Build system            n
 RPM/packaging           n
 Configuration files     n
 Startup scripts         n
 SAF services            y
 OpenSAF services        n
 Core libraries          n
 Samples                 n
 Tests                   n
 Other                   n


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

revision e71ccceccae17aa9876ab9108b62a4fc36c99140
Author:	thang.d.nguyen <tha...@de...>
Date:	Mon, 24 Jun 2024 21:36:17 +0700

smf: fix one step upgrade failed [#3354]

In large cluster or system under high load, during one step
upgrade, SMF orders AMF to lock node group(NG). There are
many request to IMM to update attribute and it causes the
timeout respond from IMM to AMF. SMF receives timeout then
retry lock again and again while the first lock still on
going. When the first lock is successful and the request
lock again from SMF will receive NO_OP error from AMF.

In this case, NO_OP should be considered as a success.



Complete diffstat:
------------------
 src/smf/smfd/SmfAdminState.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)


Testing Commands:
-----------------
N/A

Testing, Expected Results:
--------------------------
N/A

Conditions of Submission:
-------------------------
Ack from reviewer

Arch      Built     Started    Linux distro
-------------------------------------------
mips        n          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 ~/.gitconfig file (i.e. user.name, user.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.

[devel] [PATCH 1/1] smf: fix one step upgrade failed [#3354]

[thang.d.nguyen <tha...@de...>]

In large cluster or system under high load, during one step
upgrade, SMF orders AMF to lock node group(NG). There are
many request to IMM to update attribute and it causes the
timeout respond from IMM to AMF. SMF receives timeout then
retry lock again and again while the first lock still on
going. When the first lock is successful and the request
lock again from SMF will receive NO_OP error from AMF.

In this case, NO_OP should be considered as a success.
---
 src/smf/smfd/SmfAdminState.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/smf/smfd/SmfAdminState.cc b/src/smf/smfd/SmfAdminState.cc
index 958b7ae82..c20df8d74 100755
--- a/src/smf/smfd/SmfAdminState.cc
+++ b/src/smf/smfd/SmfAdminState.cc
@@ -926,6 +926,9 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
           saImmOmAdminOperationInvoke_2(ownerHandle_, &nodeGroupName, 0,
                                         adminOp, params, &oi_rc,
                                         smfd_cb->adminOpTimeout);
+      if ((imm_rc != SA_AIS_OK) || (oi_rc != SA_AIS_OK))
+        LOG_WA("%s: imm_rc: %s, oi_rc: %s", __FUNCTION__,
+            saf_error(imm_rc), saf_error(oi_rc));
       if ((imm_rc == SA_AIS_ERR_TRY_AGAIN) ||
           (imm_rc == SA_AIS_OK && oi_rc == SA_AIS_ERR_TRY_AGAIN)) {
         base::Sleep(base::MillisToTimespec(2000));
@@ -933,7 +936,8 @@ bool SmfAdminStateHandler::nodeGroupAdminOperation(
       } else if (imm_rc == SA_AIS_ERR_TIMEOUT) {
         // Retry
         continue;
-      } else if (imm_rc == SA_AIS_ERR_NO_OP) {
+      } else if ((imm_rc == SA_AIS_ERR_NO_OP) ||
+                (oi_rc == SA_AIS_ERR_NO_OP)) {
         // If an admin operation is already performed SA_AIS_ERR_NO_OP
         // is returned. Treat this as OK, just log it and return
         // operation success
-- 
2.25.1

Re: [devel] [PATCH 1/1] clm: fix error printed in syslog [#3353]

[Thien M. H. <thi...@de...>]

Hi Thang,

ACK with 4 minor comments.

Best Regards,
Thien

-----Original Message-----
From: Thang Duc Nguyen <tha...@de...> 
Sent: Saturday, May 25, 2024 6:21 PM
To: Thien Minh Huynh <thi...@de...>; Dat Tran Quoc Phan <dat...@de...>
Cc: ope...@li...; Thang Duc Nguyen <tha...@de...>
Subject: [PATCH 1/1] clm: fix error printed in syslog [#3353]

When the opensafd starts failed. The systemd tries to restart opensafd service. The CLMD did not update nodeup state correctly and it causes the error message.

The fix is to correct that state.
---
 src/clm/clmd/clms_evt.cc | 35 ++++++++++++++++++++++++++++++++++-
 src/clm/clmd/clms_evt.h  |  2 ++
 src/clm/clmd/clms_mds.cc |  4 ++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/clm/clmd/clms_evt.cc b/src/clm/clmd/clms_evt.cc index 83fe76a1a..9ad7e4d25 100644
--- a/src/clm/clmd/clms_evt.cc
+++ b/src/clm/clmd/clms_evt.cc
@@ -39,6 +39,7 @@
 
 static uint32_t process_api_evt(CLMSV_CLMS_EVT *evt);  static uint32_t proc_clma_updn_mds_msg(CLMSV_CLMS_EVT *evt);
+static uint32_t proc_clmna_updn_mds_msg(CLMSV_CLMS_EVT *evt);
 static uint32_t proc_mds_node_evt(CLMSV_CLMS_EVT *evt);  static uint32_t proc_rda_evt(CLMSV_CLMS_EVT *evt);  static uint32_t proc_mds_quiesced_ack_msg(CLMSV_CLMS_EVT *evt); @@ -77,7 +78,11 @@ static const CLMSV_CLMS_EVT_HANDLER clms_clmsv_top_level_evt_dispatch_tbl[] = {
     proc_mds_quiesced_ack_msg,
     proc_node_lock_tmr_exp_msg,
     proc_mds_node_evt,
-    proc_rda_evt};
+    proc_rda_evt,
+    nullptr,
+    nullptr,
+    proc_clmna_updn_mds_msg,
+    proc_clmna_updn_mds_msg};
 
 static const CLMSV_CLMS_CLMA_API_MSG_HANDLER clms_clma_api_msg_dispatcher[] = {
     proc_initialize_msg,   proc_finalize_msg, proc_track_start_msg,
@@ -1016,7 +1021,31 @@ done:
  * @return  NCSCC_RC_SUCCESS/NCSCC_RC_FAILURE
  *
  */

[Thien] Correct description of this function.
[Thien] Handle return code for failure case.
+static uint32_t proc_clmna_updn_mds_msg(CLMSV_CLMS_EVT *evt) {
+  TRACE_ENTER();
+  uint32_t rc = NCSCC_RC_SUCCESS;
+  SaClmNodeIdT node_id = evt->info.node_mds_info.node_id;
+  CLMS_CLUSTER_NODE *node = nullptr;
 
+  node = clms_node_get_by_id(node_id);
+  if (node == nullptr) {
+    LOG_WA("node is not exist");
+    return rc;
+  }
+
+  switch (evt->type) {
+    case CLMSV_CLMS_CLMNA_UP:
+      break;
+    case CLMSV_CLMS_CLMNA_DOWN:
+      node->nodeup = SA_FALSE;
+      break;
+    default:
+      TRACE("Unknown evt type!!!");
+      break;

[Thien] Correct indent
+    }
+  TRACE_LEAVE();
+  return rc;
+}
 static uint32_t proc_clma_updn_mds_msg(CLMSV_CLMS_EVT *evt) {
   TRACE_ENTER();
   CLMS_CKPT_REC ckpt;
@@ -1740,6 +1769,10 @@ void clms_process_mbx(SYSF_MBX *mbx) {
       clms_clmsv_top_level_evt_dispatch_tbl[msg->type](msg);
       break;

[Thien] I suggest merge CLMSV_CLMS_CLMA_DOWN and CLMSV_CLMS_CLMNA_DOWN to the same one instead of separate about case. 
+    case CLMSV_CLMS_CLMNA_DOWN:
+      clms_clmsv_top_level_evt_dispatch_tbl[msg->type](msg);
+      break;
+
     case CLSMV_CLMS_QUIESCED_ACK:
       if (clms_cb->ha_state == SA_AMF_HA_ACTIVE) {
         proc_mds_quiesced_ack_msg(msg); diff --git a/src/clm/clmd/clms_evt.h b/src/clm/clmd/clms_evt.h index 1005456f9..0e694d580 100644
--- a/src/clm/clmd/clms_evt.h
+++ b/src/clm/clmd/clms_evt.h
@@ -30,6 +30,8 @@ typedef enum clmsv_clms_evt_type {
   CLMSV_CLMS_RDA_EVT,
   CLMSV_CLMS_RT_UPDATE,
   CLMSV_AVND_DOWN_EVT,
+  CLMSV_CLMS_CLMNA_UP,
+  CLMSV_CLMS_CLMNA_DOWN,
   CLMSV_CLMS_EVT_MAX,
 } CLMSV_CLMS_EVT_TYPE;
 
diff --git a/src/clm/clmd/clms_mds.cc b/src/clm/clmd/clms_mds.cc index 24b5bcd71..82fda66de 100644
--- a/src/clm/clmd/clms_mds.cc
+++ b/src/clm/clmd/clms_mds.cc
@@ -1237,6 +1237,10 @@ static uint32_t clms_mds_svc_event(struct ncsmds_callback_info *info) {
       evt->type = CLMSV_AVND_DOWN_EVT;
       evt->info.node_mds_info.node_id = info->info.svc_evt.i_node_id;
       evt->info.node_mds_info.nodeup = SA_FALSE;
+    } else if (info->info.svc_evt.i_svc_id == NCSMDS_SVC_ID_CLMNA) {
+      evt->type = CLMSV_CLMS_CLMNA_DOWN;
+      evt->info.mds_info.node_id = info->info.svc_evt.i_node_id;
+      evt->info.mds_info.mds_dest_id = info->info.svc_evt.i_dest;
     }
     if (evt->type) {
       /* Push the event and we are done */
--
2.25.1

[devel] [PATCH 1/1] clm: fix error printed in syslog [#3353]

[thang.d.nguyen <tha...@de...>]

When the opensafd starts failed. The systemd tries to restart
opensafd service. The CLMD did not update nodeup state correctly
and it causes the error message.

The fix is to correct that state.
---
 src/clm/clmd/clms_evt.cc | 35 ++++++++++++++++++++++++++++++++++-
 src/clm/clmd/clms_evt.h  |  2 ++
 src/clm/clmd/clms_mds.cc |  4 ++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/clm/clmd/clms_evt.cc b/src/clm/clmd/clms_evt.cc
index 83fe76a1a..9ad7e4d25 100644
--- a/src/clm/clmd/clms_evt.cc
+++ b/src/clm/clmd/clms_evt.cc
@@ -39,6 +39,7 @@
 
 static uint32_t process_api_evt(CLMSV_CLMS_EVT *evt);
 static uint32_t proc_clma_updn_mds_msg(CLMSV_CLMS_EVT *evt);
+static uint32_t proc_clmna_updn_mds_msg(CLMSV_CLMS_EVT *evt);
 static uint32_t proc_mds_node_evt(CLMSV_CLMS_EVT *evt);
 static uint32_t proc_rda_evt(CLMSV_CLMS_EVT *evt);
 static uint32_t proc_mds_quiesced_ack_msg(CLMSV_CLMS_EVT *evt);
@@ -77,7 +78,11 @@ static const CLMSV_CLMS_EVT_HANDLER clms_clmsv_top_level_evt_dispatch_tbl[] = {
     proc_mds_quiesced_ack_msg,
     proc_node_lock_tmr_exp_msg,
     proc_mds_node_evt,
-    proc_rda_evt};
+    proc_rda_evt,
+    nullptr,
+    nullptr,
+    proc_clmna_updn_mds_msg,
+    proc_clmna_updn_mds_msg};
 
 static const CLMSV_CLMS_CLMA_API_MSG_HANDLER clms_clma_api_msg_dispatcher[] = {
     proc_initialize_msg,   proc_finalize_msg, proc_track_start_msg,
@@ -1016,7 +1021,31 @@ done:
  * @return  NCSCC_RC_SUCCESS/NCSCC_RC_FAILURE
  *
  */
+static uint32_t proc_clmna_updn_mds_msg(CLMSV_CLMS_EVT *evt) {
+  TRACE_ENTER();
+  uint32_t rc = NCSCC_RC_SUCCESS;
+  SaClmNodeIdT node_id = evt->info.node_mds_info.node_id;
+  CLMS_CLUSTER_NODE *node = nullptr;
 
+  node = clms_node_get_by_id(node_id);
+  if (node == nullptr) {
+    LOG_WA("node is not exist");
+    return rc;
+  }
+
+  switch (evt->type) {
+    case CLMSV_CLMS_CLMNA_UP:
+      break;
+    case CLMSV_CLMS_CLMNA_DOWN:
+      node->nodeup = SA_FALSE;
+      break;
+    default:
+      TRACE("Unknown evt type!!!");
+      break;
+    }
+  TRACE_LEAVE();
+  return rc;
+}
 static uint32_t proc_clma_updn_mds_msg(CLMSV_CLMS_EVT *evt) {
   TRACE_ENTER();
   CLMS_CKPT_REC ckpt;
@@ -1740,6 +1769,10 @@ void clms_process_mbx(SYSF_MBX *mbx) {
       clms_clmsv_top_level_evt_dispatch_tbl[msg->type](msg);
       break;
 
+    case CLMSV_CLMS_CLMNA_DOWN:
+      clms_clmsv_top_level_evt_dispatch_tbl[msg->type](msg);
+      break;
+
     case CLSMV_CLMS_QUIESCED_ACK:
       if (clms_cb->ha_state == SA_AMF_HA_ACTIVE) {
         proc_mds_quiesced_ack_msg(msg);
diff --git a/src/clm/clmd/clms_evt.h b/src/clm/clmd/clms_evt.h
index 1005456f9..0e694d580 100644
--- a/src/clm/clmd/clms_evt.h
+++ b/src/clm/clmd/clms_evt.h
@@ -30,6 +30,8 @@ typedef enum clmsv_clms_evt_type {
   CLMSV_CLMS_RDA_EVT,
   CLMSV_CLMS_RT_UPDATE,
   CLMSV_AVND_DOWN_EVT,
+  CLMSV_CLMS_CLMNA_UP,
+  CLMSV_CLMS_CLMNA_DOWN,
   CLMSV_CLMS_EVT_MAX,
 } CLMSV_CLMS_EVT_TYPE;
 
diff --git a/src/clm/clmd/clms_mds.cc b/src/clm/clmd/clms_mds.cc
index 24b5bcd71..82fda66de 100644
--- a/src/clm/clmd/clms_mds.cc
+++ b/src/clm/clmd/clms_mds.cc
@@ -1237,6 +1237,10 @@ static uint32_t clms_mds_svc_event(struct ncsmds_callback_info *info) {
       evt->type = CLMSV_AVND_DOWN_EVT;
       evt->info.node_mds_info.node_id = info->info.svc_evt.i_node_id;
       evt->info.node_mds_info.nodeup = SA_FALSE;
+    } else if (info->info.svc_evt.i_svc_id == NCSMDS_SVC_ID_CLMNA) {
+      evt->type = CLMSV_CLMS_CLMNA_DOWN;
+      evt->info.mds_info.node_id = info->info.svc_evt.i_node_id;
+      evt->info.mds_info.mds_dest_id = info->info.svc_evt.i_dest;
     }
     if (evt->type) {
       /* Push the event and we are done */
-- 
2.25.1

[devel] [PATCH 0/1] Review Request for clm: fix error printed in syslog [#3353]

[thang.d.nguyen <tha...@de...>]

Summary: clm: fix error printed in syslog [#3353]
Review request for Ticket(s): 3353
Peer Reviewer(s): *** LIST THE TECH REVIEWER(S) / MAINTAINER(S) HERE ***
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3353
Base revision: d8ad55be90183843c6946f0e29c215fba1342ee5
Personal repository: git://git.code.sf.net/u/thangng/review

--------------------------------
Impacted area       Impact y/n
--------------------------------
 Docs                    n
 Build system            n
 RPM/packaging           n
 Configuration files     n
 Startup scripts         n
 SAF services            y
 OpenSAF services        n
 Core libraries          n
 Samples                 n
 Tests                   n
 Other                   n


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

revision 0280c451d369880a25869a1e01fbd9ddc2b036a5
Author:	thang.d.nguyen <tha...@de...>
Date:	Sat, 25 May 2024 17:59:46 +0700

clm: fix error printed in syslog [#3353]

When the opensafd starts failed. The systemd tries to restart
opensafd service. The CLMD did not update nodeup state correctly
and it causes the error message.

The fix is to correct that state.



Complete diffstat:
------------------
 src/clm/clmd/clms_evt.cc | 35 ++++++++++++++++++++++++++++++++++-
 src/clm/clmd/clms_evt.h  |  2 ++
 src/clm/clmd/clms_mds.cc |  4 ++++
 3 files changed, 40 insertions(+), 1 deletion(-)


Testing Commands:
-----------------
N/A

Testing, Expected Results:
--------------------------
N/A

Conditions of Submission:
-------------------------
Ack from reviewer

Arch      Built     Started    Linux distro
-------------------------------------------
mips        n          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 ~/.gitconfig file (i.e. user.name, user.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.

Re: [devel] [PATCH 1/1] osaf: supports build with gcc/g++ 13 [#3352]

[Thien M. H. <thi...@de...>]

Hi Thang,

I see my comment inline [Thien].

Best Regards,
Thien

-----Original Message-----
From: Thang Duc Nguyen <tha...@de...> 
Sent: Monday, May 6, 2024 7:48 PM
To: Thien Minh Huynh <thi...@de...>; Dat Tran Quoc Phan <dat...@de...>
Cc: ope...@li...; Thang Duc Nguyen <tha...@de...>
Subject: [PATCH 1/1] osaf: supports build with gcc/g++ 13 [#3352]

Fix some minor errors while build/compile source code with gcc/g++ 13.
---
 src/imm/immnd/ImmModel.h           | 1 +
 src/osaf/consensus/consensus_env.h | 1 +
 src/smf/smfnd/smfnd_amf.c          | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/imm/immnd/ImmModel.h b/src/imm/immnd/ImmModel.h index 02e2178e5..3c5860cb5 100644
--- a/src/imm/immnd/ImmModel.h
+++ b/src/imm/immnd/ImmModel.h
@@ -21,6 +21,7 @@
 
 #include <saImmOm.h>
 #include <cstdarg>
+#include <cstdint>
 #include <sys/types.h>
 #include <string>
 #include <vector>
diff --git a/src/osaf/consensus/consensus_env.h b/src/osaf/consensus/consensus_env.h
index df4f93a7a..1a60b2d82 100644
--- a/src/osaf/consensus/consensus_env.h
+++ b/src/osaf/consensus/consensus_env.h
@@ -16,6 +16,7 @@
 #define OSAF_CONSENSUS_CONSENSUS_ENV_H_
 
 #include <string>
+#include <cstdint>
 #include "base/mutex.h"
 
 typedef struct _ConsensusCfg {
diff --git a/src/smf/smfnd/smfnd_amf.c b/src/smf/smfnd/smfnd_amf.c index 99a373922..787d60699 100644
--- a/src/smf/smfnd/smfnd_amf.c
+++ b/src/smf/smfnd/smfnd_amf.c
@@ -223,7 +223,7 @@ static SaAisErrorT amf_healthcheck_start(smfnd_cb_t *cb)
 	   SA_AIS_ERR_* - failure
 
 **************************************************************************/
-SaAisErrorT smfnd_amf_init(smfnd_cb_t *cb)
+uint32_t smfnd_amf_init(smfnd_cb_t *cb)


[Thien]: Why did you change from SaAisErrorT -> uint32_t during that return data declaration is SaAisErrorT.
SaAisErrorT result;

The header file change from uint32_t -> SaAisErrorT will match the data returned.

 {
 	SaAmfCallbacksT amfCallbacks;
 	SaVersionT amf_version;
--
2.25.1

[devel] [PATCH 1/1] osaf: supports build with gcc/g++ 13 [#3352]

[thang.d.nguyen <tha...@de...>]

Fix some minor errors while build/compile source code
with gcc/g++ 13.
---
 src/imm/immnd/ImmModel.h           | 1 +
 src/osaf/consensus/consensus_env.h | 1 +
 src/smf/smfnd/smfnd_amf.c          | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/imm/immnd/ImmModel.h b/src/imm/immnd/ImmModel.h
index 02e2178e5..3c5860cb5 100644
--- a/src/imm/immnd/ImmModel.h
+++ b/src/imm/immnd/ImmModel.h
@@ -21,6 +21,7 @@
 
 #include <saImmOm.h>
 #include <cstdarg>
+#include <cstdint>
 #include <sys/types.h>
 #include <string>
 #include <vector>
diff --git a/src/osaf/consensus/consensus_env.h b/src/osaf/consensus/consensus_env.h
index df4f93a7a..1a60b2d82 100644
--- a/src/osaf/consensus/consensus_env.h
+++ b/src/osaf/consensus/consensus_env.h
@@ -16,6 +16,7 @@
 #define OSAF_CONSENSUS_CONSENSUS_ENV_H_
 
 #include <string>
+#include <cstdint>
 #include "base/mutex.h"
 
 typedef struct _ConsensusCfg {
diff --git a/src/smf/smfnd/smfnd_amf.c b/src/smf/smfnd/smfnd_amf.c
index 99a373922..787d60699 100644
--- a/src/smf/smfnd/smfnd_amf.c
+++ b/src/smf/smfnd/smfnd_amf.c
@@ -223,7 +223,7 @@ static SaAisErrorT amf_healthcheck_start(smfnd_cb_t *cb)
 	   SA_AIS_ERR_* - failure
 
 **************************************************************************/
-SaAisErrorT smfnd_amf_init(smfnd_cb_t *cb)
+uint32_t smfnd_amf_init(smfnd_cb_t *cb)
 {
 	SaAmfCallbacksT amfCallbacks;
 	SaVersionT amf_version;
-- 
2.25.1

[devel] [PATCH 0/1] Review Request for osaf: supports build with gcc/g++ 13 [#3352]

[thang.d.nguyen <tha...@de...>]

Summary: osaf: supports build with gcc/g++ 13 [#3352]
Review request for Ticket(s): 3352
Peer Reviewer(s): *** LIST THE TECH REVIEWER(S) / MAINTAINER(S) HERE ***
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3352
Base revision: 7b80b26c5ac19135820854a6edc6d50a5dea82dc
Personal repository: git://git.code.sf.net/u/thangng/review

--------------------------------
Impacted area       Impact y/n
--------------------------------
 Docs                    n
 Build system            n
 RPM/packaging           n
 Configuration files     n
 Startup scripts         n
 SAF services            y
 OpenSAF services        n
 Core libraries          n
 Samples                 n
 Tests                   n
 Other                   n


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

revision e63ed820cc55055dd7519dd4adb9067144dae55a
Author:	thang.d.nguyen <tha...@de...>
Date:	Mon, 6 May 2024 19:34:33 +0700

osaf: supports build with gcc/g++ 13 [#3352]

Fix some minor errors while build/compile source code
with gcc/g++ 13.



Complete diffstat:
------------------
 src/imm/immnd/ImmModel.h           | 1 +
 src/osaf/consensus/consensus_env.h | 1 +
 src/smf/smfnd/smfnd_amf.c          | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)


Testing Commands:
-----------------
N/A

Testing, Expected Results:
--------------------------
N/A

Conditions of Submission:
-------------------------
Ack from reviewer

Arch      Built     Started    Linux distro
-------------------------------------------
mips        n          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 ~/.gitconfig file (i.e. user.name, user.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.

Re: [devel] [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

[Thang D. N. <tha...@de...>]

Looks good. Ack.

-----Original Message-----
From: Thien Minh Huynh <thi...@de...> 
Sent: Thursday, May 2, 2024 1:11 PM
To: Thang Duc Nguyen <tha...@de...>; Dat Tran Quoc Phan <dat...@de...>
Cc: ope...@li...; Thien Minh Huynh <thi...@de...>
Subject: [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

An earlier patch supporting Python 3 introduced the SaStringT class.
With str type, default SaStringT cannot be compared.

This commit is to allow compare with str class if without casting
---
 python/pyosaf/saAis.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/python/pyosaf/saAis.py b/python/pyosaf/saAis.py index 912523e69..2f2af641f 100644
--- a/python/pyosaf/saAis.py
+++ b/python/pyosaf/saAis.py
@@ -53,6 +53,30 @@ if PY3:
 				return self.value.decode('utf-8')
 			else:
 				return self.__repr__()
+
+		def __eq__(self, other):
+			if isinstance(other, str):
+				return self.value == other.encode('utf-8')
+			return super().__eq__(other)
+
+		def __ne__(self, other):
+			return not self.__eq__(other)
+
+		def __lt__(self, other):
+			if isinstance(other, str):
+				return self.value < other.encode('utf-8')
+			return super().__lt__(other)
+
+		def __gt__(self, other):
+			if isinstance(other, str):
+				return self.value > other.encode('utf-8')
+			return super().__gt__(other)
+
+		def __ge__(self, other):
+			return self.__gt__(other) or self.__eq__(other)
+
+		def __le__(self, other):
+			return self.__lt__(other) or self.__eq__(other)
 else:
 	SaStringT = ctypes.c_char_p
 
--
2.43.0

[devel] [PATCH 0/1] Review Request for pyosaf: Allow SaStringT compare with str by default [#3351] V2

[thien.m.huynh <thi...@de...>]

Summary: pyosaf: Allow SaStringT compare with str by default [#3351]
Review request for Ticket(s): 3351
Peer Reviewer(s): Thang, Dat
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3351
Base revision: 7b80b26c5ac19135820854a6edc6d50a5dea82dc
Personal repository: git://git.code.sf.net/u/thienhuynh/review

--------------------------------
Impacted area       Impact y/n
--------------------------------
 Docs                    n
 Build system            n
 RPM/packaging           n
 Configuration files     n
 Startup scripts         n
 SAF services            n
 OpenSAF services        n
 Core libraries          n
 Samples                 n
 Tests                   n
 Other                   y


Comments (indicate scope for each "y" above):
---------------------------------------------
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision 321e1d9eeac01d5259c73c88cea56e34462ebe8e
Author:	thien.m.huynh <thi...@de...>
Date:	Thu, 2 May 2024 11:25:09 +0700

pyosaf: Allow SaStringT compare with str by default [#3351]

An earlier patch supporting Python 3 introduced the SaStringT class.
With str type, default SaStringT cannot be compared.

This commit is to allow compare with str class if without casting



Complete diffstat:
------------------
 python/pyosaf/saAis.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)


Testing Commands:
-----------------
N/A

Testing, Expected Results:
--------------------------
N/A

Conditions of Submission:
-------------------------
ACK from reviewers

Arch      Built     Started    Linux distro
-------------------------------------------
mips        n          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 ~/.gitconfig file (i.e. user.name, user.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.

[devel] [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

[thien.m.huynh <thi...@de...>]

An earlier patch supporting Python 3 introduced the SaStringT class.
With str type, default SaStringT cannot be compared.

This commit is to allow compare with str class if without casting
---
 python/pyosaf/saAis.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/python/pyosaf/saAis.py b/python/pyosaf/saAis.py
index 912523e69..2f2af641f 100644
--- a/python/pyosaf/saAis.py
+++ b/python/pyosaf/saAis.py
@@ -53,6 +53,30 @@ if PY3:
 				return self.value.decode('utf-8')
 			else:
 				return self.__repr__()
+
+		def __eq__(self, other):
+			if isinstance(other, str):
+				return self.value == other.encode('utf-8')
+			return super().__eq__(other)
+
+		def __ne__(self, other):
+			return not self.__eq__(other)
+
+		def __lt__(self, other):
+			if isinstance(other, str):
+				return self.value < other.encode('utf-8')
+			return super().__lt__(other)
+
+		def __gt__(self, other):
+			if isinstance(other, str):
+				return self.value > other.encode('utf-8')
+			return super().__gt__(other)
+
+		def __ge__(self, other):
+			return self.__gt__(other) or self.__eq__(other)
+
+		def __le__(self, other):
+			return self.__lt__(other) or self.__eq__(other)
 else:
 	SaStringT = ctypes.c_char_p
 
-- 
2.43.0

Re: [devel] [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

[Thien M. H. <thi...@de...>]

Hi Thang,

Thanks for your review. I will update in the next patch.

Best Regards,
Thien
________________________________
From: Thang Duc Nguyen <tha...@de...>
Sent: Thursday, April 25, 2024 7:50 AM
To: Thien Minh Huynh <thi...@de...>; Dat Tran Quoc Phan <dat...@de...>
Cc: ope...@li... <ope...@li...>
Subject: RE: [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

Hi Thien,
It's better if other operators can be applied, e.i, <, >, ....

B.R/Thang

-----Original Message-----
From: Thien Minh Huynh <thi...@de...>
Sent: Tuesday, April 23, 2024 10:45 AM
To: Thang Duc Nguyen <tha...@de...>; Dat Tran Quoc Phan <dat...@de...>
Cc: ope...@li...; Thien Minh Huynh <thi...@de...>
Subject: [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

SaStringT class is introduced in previous commit support python3.
Default SaStringT cannot compare with other type.

This commit is to allow compare with str class if without casting
---
 python/pyosaf/saAis.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/python/pyosaf/saAis.py b/python/pyosaf/saAis.py index 912523e69..d890c6a27 100644
--- a/python/pyosaf/saAis.py
+++ b/python/pyosaf/saAis.py
@@ -53,6 +53,16 @@ if PY3:
                                 return self.value.decode('utf-8')
                         else:
                                 return self.__repr__()
+
+               def __eq__(self, other):
+                       if isinstance(other, str):
+                               return str(self).__eq__(other)
+                       return super().__eq__(other)
+
+               def __ne__(self, other):
+                       if isinstance(other, str):
+                               return str(self).__ne__(other)
+                       return super().__ne__(other)
 else:
         SaStringT = ctypes.c_char_p

--
2.43.0

Re: [devel] [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

[Thang D. N. <tha...@de...>]

Hi Thien,
It's better if other operators can be applied, e.i, <, >, ....

B.R/Thang

-----Original Message-----
From: Thien Minh Huynh <thi...@de...> 
Sent: Tuesday, April 23, 2024 10:45 AM
To: Thang Duc Nguyen <tha...@de...>; Dat Tran Quoc Phan <dat...@de...>
Cc: ope...@li...; Thien Minh Huynh <thi...@de...>
Subject: [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

SaStringT class is introduced in previous commit support python3.
Default SaStringT cannot compare with other type.

This commit is to allow compare with str class if without casting
---
 python/pyosaf/saAis.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/python/pyosaf/saAis.py b/python/pyosaf/saAis.py index 912523e69..d890c6a27 100644
--- a/python/pyosaf/saAis.py
+++ b/python/pyosaf/saAis.py
@@ -53,6 +53,16 @@ if PY3:
 				return self.value.decode('utf-8')
 			else:
 				return self.__repr__()
+
+		def __eq__(self, other):
+			if isinstance(other, str):
+				return str(self).__eq__(other)
+			return super().__eq__(other)
+
+		def __ne__(self, other):
+			if isinstance(other, str):
+				return str(self).__ne__(other)
+			return super().__ne__(other)
 else:
 	SaStringT = ctypes.c_char_p
 
--
2.43.0

[devel] [PATCH 0/1] Review Request for pyosaf: Allow SaStringT compare with str by default [#3351]

[thien.m.huynh <thi...@de...>]

Summary: pyosaf: Allow SaStringT compare with str by default [#3351]
Review request for Ticket(s): 3351
Peer Reviewer(s): Thang, Dat
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-3351
Base revision: 7b80b26c5ac19135820854a6edc6d50a5dea82dc
Personal repository: git://git.code.sf.net/u/thienhuynh/review

--------------------------------
Impacted area       Impact y/n
--------------------------------
 Docs                    n
 Build system            n
 RPM/packaging           n
 Configuration files     n
 Startup scripts         n
 SAF services            n
 OpenSAF services        n
 Core libraries          n
 Samples                 n
 Tests                   n
 Other                   y


Comments (indicate scope for each "y" above):
---------------------------------------------
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision 2793a552e9f94419fc72d58ccb767c2e0ce7ebd6
Author:	thien.m.huynh <thi...@de...>
Date:	Tue, 23 Apr 2024 10:30:57 +0700

pyosaf: Allow SaStringT compare with str by default [#3351]

SaStringT class is introduced in previous commit support python3.
Default SaStringT cannot compare with other type.

This commit is to allow compare with str class if without casting



Complete diffstat:
------------------
 python/pyosaf/saAis.py | 10 ++++++++++
 1 file changed, 10 insertions(+)


Testing Commands:
-----------------
N/A

Testing, Expected Results:
--------------------------
N/A

Conditions of Submission:
-------------------------
ACK from reviewers


Arch      Built     Started    Linux distro
-------------------------------------------
mips        n          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 ~/.gitconfig file (i.e. user.name, user.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.

[devel] [PATCH 1/1] pyosaf: Allow SaStringT compare with str by default [#3351]

[thien.m.huynh <thi...@de...>]

SaStringT class is introduced in previous commit support python3.
Default SaStringT cannot compare with other type.

This commit is to allow compare with str class if without casting
---
 python/pyosaf/saAis.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/python/pyosaf/saAis.py b/python/pyosaf/saAis.py
index 912523e69..d890c6a27 100644
--- a/python/pyosaf/saAis.py
+++ b/python/pyosaf/saAis.py
@@ -53,6 +53,16 @@ if PY3:
 				return self.value.decode('utf-8')
 			else:
 				return self.__repr__()
+
+		def __eq__(self, other):
+			if isinstance(other, str):
+				return str(self).__eq__(other)
+			return super().__eq__(other)
+
+		def __ne__(self, other):
+			if isinstance(other, str):
+				return str(self).__ne__(other)
+			return super().__ne__(other)
 else:
 	SaStringT = ctypes.c_char_p
 
-- 
2.43.0

Re: [devel] [PATCH 1/1] ntf: check existed notification before retrieving [#3350]

[Thien M. H. <thi...@de...>]

Hi Dat,

ACK from me.

Best Regards,
Thien

________________________________
From: Dat Tran Quoc Phan <dat...@de...>
Sent: Monday, April 15, 2024 5:10 PM
To: Thang Duc Nguyen <tha...@de...>; Thien Minh Huynh <thi...@de...>; Tai Huynh Nguyen <tai...@de...>
Cc: ope...@li... <ope...@li...>; Dat Tran Quoc Phan <dat...@de...>
Subject: [PATCH 1/1] ntf: check existed notification before retrieving [#3350]

Related to ticket #3349, when ntf removes an overdue notification
from logger buffer, it will try to retrieve next notification
in queue. In case there's only a single notification in queue
and it's overdue, after removing it, ntf will try to retrieve
non-existed notification, causing ntf crash.

Solution is to check if there's existed notification in queue
before retrieving it. Also, improve checking existed notification
when logd calls saLogWriteCallback.
---
 src/ntf/ntfd/NtfLogger.cc | 8 +++++---
 src/ntf/ntfd/NtfLogger.h  | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/ntf/ntfd/NtfLogger.cc b/src/ntf/ntfd/NtfLogger.cc
index 2b5c501ee..480c9e7b6 100644
--- a/src/ntf/ntfd/NtfLogger.cc
+++ b/src/ntf/ntfd/NtfLogger.cc
@@ -108,7 +108,7 @@ void saLogStreamOpenCallback(SaInvocationT invocation,

 void saLogWriteLogCallback(SaInvocationT invocation, SaAisErrorT error) {
   TRACE_ENTER2("Callback for notificationId %llu", invocation);
-  if (NtfAdmin::theNtfAdmin->logger.getFrontNotificationId() != invocation) {
+  if (!NtfAdmin::theNtfAdmin->logger.isExistNotification(invocation)) {
     TRACE("Notification had been processed by logd, but Id is not existed"
           " in logger. Probably due to notification overdue, ignore "
           "notificationId %llu", invocation);
@@ -384,8 +384,9 @@ void NtfLogger::disableAckWaiting() {
   notification->setWaitingAck(false);
 }

-SaNtfIdentifierT NtfLogger::getFrontNotificationId() {
-  return (queuedNotificationList.front()->getNotificationId());
+bool NtfLogger::isExistNotification(SaInvocationT invocation) {
+  if (isLoggerBufferEmpty()) return false;
+  return (queuedNotificationList.front()->getNotificationId() == invocation);
 }

 void NtfLogger::logQueuedNotification() {
@@ -397,6 +398,7 @@ void NtfLogger::logQueuedNotification() {
       dequeueNotification();
       resetLoggerBufferFullFlag();
       sendLoggedConfirm(notification->getNotificationId());
+      if (isLoggerBufferEmpty()) return;
       notification = queuedNotificationList.front();
     }
     if (notification->isWaitingAck()) return;
diff --git a/src/ntf/ntfd/NtfLogger.h b/src/ntf/ntfd/NtfLogger.h
index 283fd3fe2..4af8fc3d8 100644
--- a/src/ntf/ntfd/NtfLogger.h
+++ b/src/ntf/ntfd/NtfLogger.h
@@ -73,7 +73,7 @@ class NtfLogger {
   void logQueuedNotification();
   bool isLoggerBufferEmpty() { return queuedNotificationList.empty(); }
   void disableAckWaiting();
-  SaNtfIdentifierT getFrontNotificationId();
+  bool isExistNotification(SaInvocationT invocation);

  private:
   SaAisErrorT initLog();
--
2.25.1

Re: [devel] [PATCH 1/1] ntf: check existed notification before retrieving [#3350]

[Thien M. H. <thi...@de...>]

Hi Dat,

See  my comment inline.

Best Regards,
Thien

________________________________
From: Dat Tran Quoc Phan <dat...@de...>
Sent: Monday, April 15, 2024 9:38 AM
To: Thang Duc Nguyen <tha...@de...>; Thien Minh Huynh <thi...@de...>; Tai Huynh Nguyen <tai...@de...>
Cc: ope...@li... <ope...@li...>; Dat Tran Quoc Phan <dat...@de...>
Subject: [PATCH 1/1] ntf: check existed notification before retrieving [#3350]

Related to ticket #3349, when ntf removes an overdue notification
from logger buffer, it will try to retrieve next notification
in queue. In case there's only a single notification in queue
and it's overdue, after removing it, ntf will try to retrieve
non-existed notification, causing ntf crash.

Solution is to check if there's existed notification
in queue before retrieving it. Also, checking existed
notification when logd calls saLogWriteCallback.
---
 src/ntf/ntfd/NtfLogger.cc | 8 +++++---
 src/ntf/ntfd/NtfLogger.h  | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/ntf/ntfd/NtfLogger.cc b/src/ntf/ntfd/NtfLogger.cc
index 2b5c501ee..a6ca87d75 100644
--- a/src/ntf/ntfd/NtfLogger.cc
+++ b/src/ntf/ntfd/NtfLogger.cc
@@ -108,7 +108,7 @@ void saLogStreamOpenCallback(SaInvocationT invocation,

 void saLogWriteLogCallback(SaInvocationT invocation, SaAisErrorT error) {
   TRACE_ENTER2("Callback for notificationId %llu", invocation);
-  if (NtfAdmin::theNtfAdmin->logger.getFrontNotificationId() != invocation) {
+  if (!NtfAdmin::theNtfAdmin->logger.isExistNotification(invocation)) {
     TRACE("Notification had been processed by logd, but Id is not existed"
           " in logger. Probably due to notification overdue, ignore "
           "notificationId %llu", invocation);
@@ -384,8 +384,9 @@ void NtfLogger::disableAckWaiting() {
   notification->setWaitingAck(false);
 }

-SaNtfIdentifierT NtfLogger::getFrontNotificationId() {
-  return (queuedNotificationList.front()->getNotificationId());
+bool NtfLogger::isExistNotification(SaInvocationT invocation) {
+  return (!isLoggerBufferEmpty() &&
+          (queuedNotificationList.front()->getNotificationId() == invocation));
[Thien]: I suggest break down condition to easy read
if (isLoggerBufferEmpty()) return false;
return queuedNotificationList.front()->getNotificationId() == invocation;
 }

 void NtfLogger::logQueuedNotification() {
@@ -397,6 +398,7 @@ void NtfLogger::logQueuedNotification() {
       dequeueNotification();
       resetLoggerBufferFullFlag();
       sendLoggedConfirm(notification->getNotificationId());
+      if (isLoggerBufferEmpty()) return;
       notification = queuedNotificationList.front();
     }
     if (notification->isWaitingAck()) return;
diff --git a/src/ntf/ntfd/NtfLogger.h b/src/ntf/ntfd/NtfLogger.h
index 283fd3fe2..4af8fc3d8 100644
--- a/src/ntf/ntfd/NtfLogger.h
+++ b/src/ntf/ntfd/NtfLogger.h
@@ -73,7 +73,7 @@ class NtfLogger {
   void logQueuedNotification();
   bool isLoggerBufferEmpty() { return queuedNotificationList.empty(); }
   void disableAckWaiting();
-  SaNtfIdentifierT getFrontNotificationId();
+  bool isExistNotification(SaInvocationT invocation);

  private:
   SaAisErrorT initLog();
--
2.25.1