Hi Hung,

comments inline.

Thanks,
Neel.

On 2016/07/29 04:32 PM, Hung Nguyen wrote:
> Hi Neel,
>
> Reviewed and tested the patch.
> Ack with minor comments below.
>
> BR,
> Hung Nguyen - DEK Technologies
>
> --------------------------------------------------------------------------------
> From: Neelakanta [email protected]
> Sent: Tuesday, July 26, 2016 6:20PM
> To: Zoran Milinkovic, Hung Nguyen
>      [email protected],[email protected]
> Cc: Opensaf-devel
>      [email protected]
> Subject: [PATCH 1 of 1] imm:send 2PBE preload information for controller 
> IMMND [#1925]
>
>
>   osaf/services/saf/immsv/README.2PBE        |   7 +++++
>   osaf/services/saf/immsv/immd/immd_evt.c    |  38 
> +++++++++++++++++++++++++++--
>   osaf/services/saf/immsv/immnd/immnd_cb.h   |   1 +
>   osaf/services/saf/immsv/immnd/immnd_evt.c  |   2 +-
>   osaf/services/saf/immsv/immnd/immnd_main.c |  22 +++++++++++++++++
>   5 files changed, 66 insertions(+), 4 deletions(-)
>
>
> with the #79 patch, the mds_register of standby is delayed until amfd gives 
> role.
> Because of this the standby IMMND is not able to send pr-load information.
>
> The patch sends preload information by checking node_type. If the node_type is
> controller the preload information is sent.
>
> diff --git a/osaf/services/saf/immsv/README.2PBE 
> b/osaf/services/saf/immsv/README.2PBE
> --- a/osaf/services/saf/immsv/README.2PBE
> +++ b/osaf/services/saf/immsv/README.2PBE
> @@ -175,3 +175,10 @@ toggle this flag on.
>   For "normal", but unplanned, processor restarts, we recommend that this 
> flag not be
>   toggled on. This means that for such processor restarts, persistent writes 
> will not be
>   allowed untill both SCs are available again.
> +
> +2PBE with spares(#79 & #1925)
> +----------------------------
> +From OpenSAG 5.0 the mds_register for IMMD is delayed until amfd comes up 
> and gives role.
> +Because of this the standby role is delayed. There is a chance that chosen 
> standby immnd
> +can be from different node than the actual node which got standby role. It 
> is not
> +recommended to configure 2PBE with spares.
> diff --git a/osaf/services/saf/immsv/immd/immd_evt.c 
> b/osaf/services/saf/immsv/immd/immd_evt.c
> --- a/osaf/services/saf/immsv/immd/immd_evt.c
> +++ b/osaf/services/saf/immsv/immd/immd_evt.c
> @@ -732,7 +732,11 @@ static void immd_accept_node(IMMD_CB *cb
>               cb->immnd_coord = node_info->immnd_key;
>               cb->payload_coord_dest = node_info->immnd_dest;
>               node_info->isCoord = true;
> -     }
> +     } else if(cb->immnd_coord == 0 && cb->mIs2Pbe){
> +             LOG_NO("IMMND found at %x Cluster is loading. 2PBE configured 
> => Wait.", node_info->immnd_key);
> +             accept_evt.info.immnd.info.ctrl.canBeCoord = 2; /* 2PBE => 
> order preload. */
> +        }
> +
>   
>       if (node_info->isCoord) {
>               accept_evt.info.immnd.info.ctrl.isCoord = true;
> @@ -1979,8 +1983,18 @@ static uint32_t immd_evt_proc_2pbe_prelo
>                       evt->info.pbe2.epoch, evt->info.pbe2.maxCcbId,
>                       evt->info.pbe2.maxCommitTime);
>               cb->remPbe = evt->info.pbe2;
> +     } else if (!cb->is_rem_immnd_up && !cb->immd_remote_up) {
> +             LOG_NO("2PBE preload info from first remote SC can be standby 
> Epoch: %u MaxCcb:%u MaxTime%u",
> +                             evt->info.pbe2.epoch, evt->info.pbe2.maxCcbId,
> +                             evt->info.pbe2.maxCommitTime);
> +             cb->is_rem_immnd_up = true;
> +             cb->rem_immnd_dest = sinfo->dest;
> +             cb->remPbe = evt->info.pbe2;
> +     } else if (cb->is_rem_immnd_up){
> +             LOG_NO("2PBE preload info from remote SC may be from spare will 
> be discarded");
>       }
>   
> +
>       if(cb->m2PbeCanLoad) {
>               LOG_NO("m2PbeCanLoad already set (timeout ?)");
>       } else {
> @@ -2632,6 +2646,7 @@ static uint32_t immd_evt_proc_mds_evt(IM
>               TRACE_5("Process MDS EVT NCSMDS_RED_UP, my PID:%u", getpid());
>               if (cb->node_id != mds_info->node_id) {
>                       MDS_DEST tmpDest = 0LL;
> +                     uint32_t immnd_remote_id = 0;
>                       TRACE_5("Remote IMMD is UP.");
>   
>                       cb->immd_remote_id = 
> immd_get_slot_and_subslot_id_from_node_id(mds_info->node_id);
> @@ -2652,16 +2667,33 @@ static uint32_t immd_evt_proc_mds_evt(IM
>   
>                                               node_info->isOnController = 
> true;
>                                               TRACE_5("Located STDBY IMMND =  
> %x node_id:%x",
> -                                                     
> immd_get_slot_and_subslot_id_from_node_id(mds_info->node_id),
> +                                             
> immd_get_slot_and_subslot_id_from_node_id(mds_info->node_id),
>                                                       mds_info->node_id);
> -                                             immd_accept_node(cb, node_info, 
> true, false); /* <==== Can not be sc-absence veteran if on sc. */
> +                                             if(!cb->is_rem_immnd_up){
> +                                                     immd_accept_node(cb, 
> node_info, true, false); /* <==== Can not be sc-absence veteran if on sc. */
> +                                             } if (cb->is_rem_immnd_up && 
> cb->mIs2Pbe){
> +                                                     immnd_remote_id =
> +                                                          
> immd_get_slot_and_subslot_id_from_mds_dest(node_info->immnd_dest);
> +                                             }
> +                                             if(cb->is_rem_immnd_up && 
> cb->mIs2Pbe){ /* The immnd and immd must be from same node*/
> +                                                     TRACE("The 
> immd_remote_id =%d immnd_remote_id=%d",
> +                                                             
> cb->immd_remote_id, immnd_remote_id);
> +                                                     
> osafassert(cb->immd_remote_id == immnd_remote_id);
> +                                             }
>                                       }
> +
>                                       /* Break out of while-1. We found */
>                                       break;
>                               }
>                               tmpDest = node_info->immnd_dest;
>                               immd_immnd_info_node_getnext(&cb->immnd_tree, 
> &tmpDest, &node_info);
>                       }
> +                     /* From OpenSAF 5.1 with #79 the mds_register for immd 
> will happen after amfd started.
> +                        Ihe IMMND should have started.synced or loaded 
> before the IMMD registers #1925
> +                     */
> +                     if ( cb->mIs2Pbe && cb->mds_role == V_DEST_RL_STANDBY){
> +                             cb->m2PbeCanLoad = true;
> +                     }
>               }
>               break;
>   
> diff --git a/osaf/services/saf/immsv/immnd/immnd_cb.h 
> b/osaf/services/saf/immsv/immnd/immnd_cb.h
> --- a/osaf/services/saf/immsv/immnd/immnd_cb.h
> +++ b/osaf/services/saf/immsv/immnd/immnd_cb.h
> @@ -177,6 +177,7 @@ typedef struct immnd_cb_tag {
>       NCS_SEL_OBJ usr1_sel_obj;       /* Selection object for USR1 signal 
> events */
>       SaSelectionObjectT amf_sel_obj; /* Selection Object for AMF events */
>       int nid_started;        /* true if started by NID */
> +     bool isNodeTypeController; // true node type is controller
>   } IMMND_CB;
>   
>   /* CB prototypes */
> diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c 
> b/osaf/services/saf/immsv/immnd/immnd_evt.c
> --- a/osaf/services/saf/immsv/immnd/immnd_evt.c
> +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c
> @@ -8936,7 +8936,7 @@ static uint32_t immnd_evt_proc_intro_rsp
>               }
>   
>               cb->mCanBeCoord = evt->info.ctrl.canBeCoord;
> -             if((cb->mCanBeCoord == 2) && (cb->m2Pbe < 2)) {
> +             if((cb->mCanBeCoord == 2) && (cb->m2Pbe < 2) && 
> immnd_cb->isNodeTypeController) {
>                       LOG_NO("2PBE startup arbitration initiated from IMMD");
>                       cb->m2Pbe = 2;
>                       /* mCanBeCoord > 0  => This immnd resides on an SC. */
>
> [Hung] If cb->mCanBeCoord is 2 and immnd_cb->isNodeTypeController is false, 
> we should set cb->mCanBeCoord back to 0.
yes, this can be done.
will add while pushing the patch.
>
> diff --git a/osaf/services/saf/immsv/immnd/immnd_main.c 
> b/osaf/services/saf/immsv/immnd/immnd_main.c
> --- a/osaf/services/saf/immsv/immnd/immnd_main.c
> +++ b/osaf/services/saf/immsv/immnd/immnd_main.c
> @@ -168,6 +168,28 @@ static uint32_t immnd_initialize(char *p
>               LOG_NO("Persistent Back-End capability configured, Pbefile:%s  
> (suffix may get added)",
>                       immnd_cb->mPbeFile);
>       }
> +     
> +     FILE *fp;
> +     char node_type[20];
> +     fp = fopen(PKGSYSCONFDIR "/node_type", "r");
> +     if (fp == NULL) {
> +                LOG_ER("Could not open file %s - %s", PKGSYSCONFDIR 
> "/node_type", strerror(errno));
> +                goto done;
> +        }
> +     if(EOF == fscanf(fp, "%s", node_type)) {
> +                LOG_ER("Could not read node type - %s", strerror(errno));
> +                fclose(fp);
> +                goto done;
> +        }
> +     if (strcmp(node_type,"controller")==0){
> +             immnd_cb->isNodeTypeController = true;
> +     } else if (strcmp(node_type,"payload")==0){
> +             immnd_cb->isNodeTypeController = false;
> +     } else {
> +             LOG_ER("Wrong node type is specified for the node");
>
> [Hung] fclose() before goto done.
will add, while pushing the patch.
>
> +             goto done;
> +     }
> +     fclose(fp);
>
> [Hung] I think we should create a function for checking node type and put it 
> to somewhere like nid_api.c
> It might be used by other services in future.
>   
It is not advisable, determine the node type by reading node_type.
In this we do not have other approach.
>       immnd_cb->mRim = SA_IMM_INIT_FROM_FILE;
>       immnd_cb->mPbeVeteran = SA_FALSE;
>

------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to