Ack (reviewed only)
Minor code comments inline
Thanks,
Hans
> -----Original Message-----
> From: [email protected] [mailto:[email protected]]
> Sent: den 12 september 2014 00:15
> To: Hans Feldt; [email protected]
> Cc: [email protected]
> Subject: [PATCH 1 of 1] amfd : send state change notification for error
> reported on comp [#1058]
>
> osaf/services/saf/amf/amfd/include/ntf.h | 9 ++
> osaf/services/saf/amf/amfd/ntf.cc | 124
> +++++++++++++++++-------------
> 2 files changed, 78 insertions(+), 55 deletions(-)
>
>
> AMF sends error report and error clear notifications as alarms.
> This patch changes this notification to state change notification.
>
> diff --git a/osaf/services/saf/amf/amfd/include/ntf.h
> b/osaf/services/saf/amf/amfd/include/ntf.h
> --- a/osaf/services/saf/amf/amfd/include/ntf.h
> +++ b/osaf/services/saf/amf/amfd/include/ntf.h
> @@ -34,6 +34,15 @@
> #define ADDITION_TEXT_LENGTH 320
> #define AMF_NTF_SENDER "safApp=safAmfService"
>
> +/* For presence of additional inf in notification */
> +#define ADDITIONAL_INFO_PRESENT true
> +#define ADDITIONAL_INFO_NOT_PRESENT false
[Hans] not sure why this is needed, just use true/false directly
> +
> +/* All states like oper, readiness etc starts from 1, so defining not
> applicable values */
> +#define STATE_ID_NA 0
> +#define OLD_STATE_NA 0
> +#define NEW_STATE_NA 0
> +
> /* Alarms */
> void avd_send_comp_inst_failed_alarm(const SaNameT *comp_name, const SaNameT
> *node_name);
> void avd_send_comp_clean_failed_alarm(const SaNameT *comp_name, const
> SaNameT *node_name);
> diff --git a/osaf/services/saf/amf/amfd/ntf.cc
> b/osaf/services/saf/amf/amfd/ntf.cc
> --- a/osaf/services/saf/amf/amfd/ntf.cc
> +++ b/osaf/services/saf/amf/amfd/ntf.cc
> @@ -55,7 +55,7 @@ void avd_send_comp_inst_failed_alarm(con
> SA_NTF_SOFTWARE_ERROR,
> SA_NTF_SEVERITY_MAJOR,
> (NCSCONTEXT)node_name,
> - 1 /* add_info is node_name */);
> + ADDITIONAL_INFO_PRESENT /* add_info is
> node_name */);
> }
>
>
> /*****************************************************************************
> @@ -87,7 +87,7 @@ void avd_send_comp_clean_failed_alarm(co
> SA_NTF_SOFTWARE_ERROR,
> SA_NTF_SEVERITY_MAJOR,
> (NCSCONTEXT)node_name,
> - 1 /* add_info is node_name */);
> + ADDITIONAL_INFO_PRESENT /* add_info is
> node_name */);
>
> }
>
> /*****************************************************************************
> @@ -121,7 +121,7 @@ void avd_send_cluster_reset_alarm(const
> SA_NTF_SOFTWARE_ERROR,
> SA_NTF_SEVERITY_MAJOR,
> NULL,
> - 0 /* No add_info */);
> + ADDITIONAL_INFO_NOT_PRESENT /* No add_info */);
>
> }
>
> @@ -152,7 +152,7 @@ void avd_send_si_unassigned_alarm(const
> SA_NTF_SOFTWARE_ERROR,
> SA_NTF_SEVERITY_MAJOR,
> NULL,
> - 0 /* No add_info */);
> + ADDITIONAL_INFO_NOT_PRESENT /* No add_info */);
>
> }
>
> @@ -184,7 +184,7 @@ void avd_send_comp_proxy_status_unproxie
> SA_NTF_SOFTWARE_ERROR,
> SA_NTF_SEVERITY_MAJOR,
> NULL,
> - 0 /* No add_info */);
> + ADDITIONAL_INFO_NOT_PRESENT /* No add_info */);
>
> }
>
> @@ -220,7 +220,7 @@ void avd_send_admin_state_chg_ntf(const
> old_state,
> new_state,
> NULL,
> - 0);
> + ADDITIONAL_INFO_NOT_PRESENT);
>
> }
>
> @@ -256,7 +256,7 @@ void avd_send_oper_chg_ntf(const SaNameT
> old_state,
> new_state,
> NULL,
> - 0);
> + ADDITIONAL_INFO_NOT_PRESENT);
> }
>
>
> /*****************************************************************************
> @@ -291,7 +291,7 @@ void avd_send_su_pres_state_chg_ntf(cons
> old_state,
> new_state,
> NULL,
> - 0);
> + ADDITIONAL_INFO_NOT_PRESENT);
>
> }
>
> @@ -330,7 +330,7 @@ void avd_send_su_ha_state_chg_ntf(const
> old_state,
> new_state,
> (NCSCONTEXT)si_name,
> - 2 /* Si_name */);
> + ADDITIONAL_INFO_PRESENT /* Si_name */);
>
> }
>
> @@ -367,7 +367,7 @@ void avd_send_su_ha_readiness_state_chg_
> old_state,
> new_state,
> (NCSCONTEXT)si_name,
> - 2 /* Si_name */);
> + ADDITIONAL_INFO_PRESENT /* Si_name */);
>
> }
>
> @@ -402,7 +402,7 @@ void avd_send_si_assigned_ntf(const SaNa
> old_state,
> new_state,
> NULL,
> - 0);
> + ADDITIONAL_INFO_NOT_PRESENT);
>
> }
>
> @@ -439,7 +439,7 @@ void avd_send_comp_proxy_status_proxied_
> old_status,
> new_status,
> NULL,
> - 0);
> + ADDITIONAL_INFO_NOT_PRESENT);
>
> }
>
> @@ -472,7 +472,7 @@ void avd_alarm_clear(const SaNameT *name
> probableCause,
> SA_NTF_SEVERITY_CLEARED,
> NULL,
> - 0);
> + ADDITIONAL_INFO_NOT_PRESENT);
> }
>
> SaAisErrorT fill_ntf_header_part_avd(SaNtfNotificationHeaderT
> *notificationHeader,
> @@ -483,7 +483,7 @@ SaAisErrorT fill_ntf_header_part_avd(SaN
> SaUint16T minorId,
> SaInt8T *avd_name,
> NCSCONTEXT add_info,
> - int type,
> + int is_additional_info_present,
[Hans] should be a bool, "additional_info_is_present" is a better name
> SaNtfNotificationHandleT notificationHandle)
> {
>
> @@ -503,30 +503,31 @@ SaAisErrorT fill_ntf_header_part_avd(SaN
> (void)strcpy(notificationHeader->additionalText, (SaInt8T*)add_text);
>
> /* Fill the additional info if present */
> - if (type != 0) {
> - switch (minorId) {
> - case SA_AMF_NTFID_ERROR_REPORT:
> - SaNtfAdditionalInfoT *info;
> - info = (SaNtfAdditionalInfoT *) add_info;
> - notificationHeader->additionalInfo[0].infoId =
> info->infoId;
> - notificationHeader->additionalInfo[0].infoType =
> info->infoType;
> - notificationHeader->additionalInfo[0].infoValue =
> info->infoValue;
> - break;
> - default:
> + if (is_additional_info_present == ADDITIONAL_INFO_PRESENT) {
[Hans] becomes "== true" instead
> + if (minorId == SA_AMF_NTFID_ERROR_REPORT) {
> + SaAmfRecommendedRecoveryT *recovery =
> (SaAmfRecommendedRecoveryT *) (add_info);
> + notificationHeader->additionalInfo[0].infoId =
> SA_AMF_AI_APPLIED_RECOVERY;
> + notificationHeader->additionalInfo[0].infoType =
> SA_NTF_VALUE_UINT64;
> +
> notificationHeader->additionalInfo[0].infoValue.uint64Val = *recovery;
> + } else {
> SaStringT dest_ptr;
> SaAisErrorT ret;
> SaNameT *name = (SaNameT*)(add_info);
> - if (type == 1) {
> +
> + if ((minorId == SA_AMF_NTFID_COMP_INSTANTIATION_FAILED)
> ||
> + (minorId ==
> SA_AMF_NTFID_COMP_CLEANUP_FAILED)) {
> /* node_name */
> notificationHeader->additionalInfo[0].infoId =
> SA_AMF_NODE_NAME;
> notificationHeader->additionalInfo[0].infoType
> = SA_NTF_VALUE_LDAP_NAME;
>
> - } else if (type == 2) {
> + } else if ((minorId == SA_AMF_NTFID_SU_SI_HA_STATE) ||
> + (minorId ==
> SA_AMF_NTFID_SU_SI_HA_READINESS_STATE)) {
> /* si_name */
> notificationHeader->additionalInfo[0].infoId =
> SA_AMF_SI_NAME;
> notificationHeader->additionalInfo[0].infoType
> = SA_NTF_VALUE_LDAP_NAME;
>
> }
> +
> ret = saNtfPtrValAllocate(notificationHandle,
> sizeof (SaNameT) + 1,
> (void**)&dest_ptr,
> @@ -538,7 +539,6 @@ SaAisErrorT fill_ntf_header_part_avd(SaN
> }
>
> memcpy(dest_ptr, name, sizeof(SaNameT));
> - break;
> }
> }
> return SA_AIS_OK;
> @@ -641,22 +641,29 @@ uint32_t sendStateChangeNotificationAvd(
> SaUint16T oldstate,
> SaUint16T newState,
> NCSCONTEXT add_info,
> - int type)
> + int is_additionalInfo_present)
> {
> uint32_t status = NCSCC_RC_FAILURE;
> SaNtfStateChangeNotificationT myStateNotification;
> SaUint16T add_info_items = 0;
> SaUint64T allocation_size = 0;
> + SaUint16T num_of_changedStates = 1;
>
> if (!avd_cb->active_services_exist) {
> // TODO #3051
> LOG_WA("State change notification lost for '%s'",
> ntf_object.value);
> return status;
> }
> -
> - if (type != 0) {
> + if (is_additionalInfo_present == ADDITIONAL_INFO_PRESENT) {
> add_info_items = 1;
> allocation_size = SA_NTF_ALLOC_SYSTEM_LIMIT;
> + } else {
> + add_info_items = 0;
> + allocation_size = 0;
> + }
> +
> + if (stateId == STATE_ID_NA) {
> + num_of_changedStates = 0;
> }
>
> status = saNtfStateChangeNotificationAllocate(avd_cb->ntfHandle,/*
> handle to Notification Service instance */
> @@ -668,7 +675,7 @@ uint32_t sendStateChangeNotificationAvd(
> /* number of additional
> info items */
> add_info_items,
> /* number of state
> changes */
> - 1,
> + num_of_changedStates,
> /* use default allocation
> size */
> allocation_size);
>
> @@ -685,7 +692,7 @@ uint32_t sendStateChangeNotificationAvd(
> minorId,
> const_cast<SaInt8T*>(AMF_NTF_SENDER),
> add_info,
> - type,
> + is_additionalInfo_present,
> myStateNotification.notificationHandle);
>
> if (status != SA_AIS_OK) {
> @@ -695,10 +702,13 @@ uint32_t sendStateChangeNotificationAvd(
> }
>
> *(myStateNotification.sourceIndicator) =
> static_cast<SaNtfSourceIndicatorT>(sourceIndicator);
> - myStateNotification.changedStates->stateId = stateId;
> - myStateNotification.changedStates->oldStatePresent = SA_TRUE;
> - myStateNotification.changedStates->oldState = oldstate;
> - myStateNotification.changedStates->newState = newState;
> +
> + if (num_of_changedStates == 1) {
> + myStateNotification.changedStates->stateId = stateId;
> + myStateNotification.changedStates->oldStatePresent = SA_TRUE;
> + myStateNotification.changedStates->oldState = oldstate;
> + myStateNotification.changedStates->newState = newState;
> + }
>
> status = saNtfNotificationSend(myStateNotification.notificationHandle);
>
> @@ -724,28 +734,32 @@ void avd_send_error_report_ntf(const SaN
> {
>
> TRACE_ENTER();
> - if ((recovery > SA_AMF_NO_RECOMMENDATION) && (recovery <
> SA_AMF_CONTAINER_RESTART)) {
> - char add_text[ADDITION_TEXT_LENGTH];
> - SaNtfAdditionalInfoT add_info;
> - add_info.infoId = SA_AMF_AI_RECOMMENDED_RECOVERY;
> - add_info.infoType = SA_NTF_VALUE_UINT64;
> - add_info.infoValue.uint64Val = recovery;
> + char add_text[ADDITION_TEXT_LENGTH];
> + SaAmfNotificationMinorIdT minorid;
> + bool is_additionalInfo_present;
> +
> + if ((recovery >= SA_AMF_NO_RECOMMENDATION) && (recovery <
> SA_AMF_CONTAINER_RESTART)) {
> snprintf(add_text, ADDITION_TEXT_LENGTH, "Error reported on %s
> with recovery %s", name->value,
> amf_recovery[recovery]);
> - sendAlarmNotificationAvd(avd_cb,
> - *name,
> - (SaUint8T*)add_text,
> - SA_SVC_AMF,
> - SA_AMF_NTFID_ERROR_REPORT,
> - SA_NTF_SOFTWARE_ERROR,
> - SA_NTF_SEVERITY_MAJOR,
> - (NCSCONTEXT) &add_info,
> - 1 /* No add_info */);
> + minorid = SA_AMF_NTFID_ERROR_REPORT;
> + is_additionalInfo_present = ADDITIONAL_INFO_PRESENT;
> + } else {
> + snprintf(add_text, ADDITION_TEXT_LENGTH, "Error reported on %s
> is now cleared", name->value);
> + minorid = SA_AMF_NTFID_ERROR_CLEAR;
> + is_additionalInfo_present = ADDITIONAL_INFO_NOT_PRESENT;
> + }
>
> - } else {
> - avd_alarm_clear(name, SA_AMF_NTFID_ERROR_CLEAR,
> - SA_NTF_SOFTWARE_ERROR);
> - }
> + sendStateChangeNotificationAvd(avd_cb,
> + *name,
> + (SaUint8T*)add_text,
> + SA_SVC_AMF,
> + minorid,
> + SA_NTF_UNKNOWN_OPERATION,
> + STATE_ID_NA,
> + OLD_STATE_NA,
> + NEW_STATE_NA,
> + (NCSCONTEXT)&recovery,
> + is_additionalInfo_present);
>
> TRACE_LEAVE();
> }
------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
