Actually I found  a much simpler fix, just set "num-pref" to ~0(-1)
instead of su_cnt. That way no recalculation is needed at all.
I will share a new patch, v4 I guess :-)
Thanks,
Hans

On 18 April 2014 13:18, Nagendra Kumar <[email protected]> wrote:
> I have concerns...
>
> 1.      The below attributes are added newly, should they not be checkpointed 
> from Act to Standby ?
>
> +       bool saAmfSGNumPrefInserviceSUs_configured;
> +       bool saAmfSGNumPrefAssignedSUs_configured;
>
> 2.      During upgrade, saAmfSGNumPrefInserviceSUs on Act will differ from 
> Standby as on new it will be calculated by adjust_num_preferred() and on old, 
> it     will be calculated by sg_su_cnt().
>         How can we mitigate it ?
>
> Thanks
> -Nagu
>
>> -----Original Message-----
>> From: Hans Feldt [mailto:[email protected]]
>> Sent: 10 March 2014 22:05
>> To: Praveen Malviya
>> Cc: [email protected]
>> Subject: [devel] [PATCH 1 of 1] v2 amfd: fix SG reconfig when SUs are
>> added/removed [#278]
>>
>>  osaf/services/saf/amf/amfd/include/sg.h |   6 ++-
>>  osaf/services/saf/amf/amfd/sg.cc        |  80 
>> ++++++++++++++++++++------------
>>  2 files changed, 54 insertions(+), 32 deletions(-)
>>
>>
>> Problem: When an SG is extended with a new SU, it will not be instatiated or
>> assigned.
>>
>> Reason: SG attributes saAmfSGNumPrefAssignedSUs and
>> saAmfSGNumPrefInserviceSUs
>> are not calculated dynamically as SUs are added (and deleted).
>>
>> Change: Add a shadow bool "xxx_configured" member of the SG struct
>> (following the same pattern as for SGautorepair). If the value of this member
>> is false, the value of the corresponding attribute will be calculated.
>>
>> diff --git a/osaf/services/saf/amf/amfd/include/sg.h
>> b/osaf/services/saf/amf/amfd/include/sg.h
>> --- a/osaf/services/saf/amf/amfd/include/sg.h
>> +++ b/osaf/services/saf/amf/amfd/include/sg.h
>> @@ -69,7 +69,11 @@ typedef struct avd_sg_tag {
>>                                * Checkpointing - Sent as a one time update.
>>                                */
>>
>> -     bool saAmfSGAutoRepair_configured; /* True when user configures
>> saAmfSGAutoRepair else false */
>> +     /* True when the corresponding configuration attribute is configured */
>> +     bool saAmfSGAutoRepair_configured;
>> +     bool saAmfSGNumPrefInserviceSUs_configured;
>> +     bool saAmfSGNumPrefAssignedSUs_configured;
>> +
>>     /******************** B.04 model
>> *************************************************/
>>       SaNameT saAmfSGType;    /* Network order. */
>>       SaNameT saAmfSGSuHostNodeGroup; /* Network order. */
>> diff --git a/osaf/services/saf/amf/amfd/sg.cc
>> b/osaf/services/saf/amf/amfd/sg.cc
>> --- a/osaf/services/saf/amf/amfd/sg.cc
>> +++ b/osaf/services/saf/amf/amfd/sg.cc
>> @@ -308,12 +308,14 @@ static AVD_SG *sg_create(const SaNameT *
>>               sg->saAmfSGNumPrefStandbySUs = 1;
>>       }
>>
>> -     if
>> (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGNumPrefInserviceS
>> Us"), attributes, 0, &sg->saAmfSGNumPrefInserviceSUs) != SA_AIS_OK) {
>> -             /* empty => later assign number of SUs */
>> +     if
>> (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGNumPrefInserviceS
>> Us"),
>> +                     attributes, 0, &sg->saAmfSGNumPrefInserviceSUs) ==
>> SA_AIS_OK) {
>> +             sg->saAmfSGNumPrefInserviceSUs_configured = true;
>>       }
>>
>> -     if
>> (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGNumPrefAssignedSU
>> s"), attributes, 0, &sg->saAmfSGNumPrefAssignedSUs) != SA_AIS_OK) {
>> -             /* empty => later assign to saAmfSGNumPrefInserviceSUs */
>> +     if
>> (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfSGNumPrefAssignedSU
>> s"),
>> +                     attributes, 0, &sg->saAmfSGNumPrefAssignedSUs) ==
>> SA_AIS_OK) {
>> +             sg->saAmfSGNumPrefAssignedSUs_configured = true;
>>       }
>>
>>       sg->saAmfSGMaxActiveSIsperSU = -1; // magic number for no limit
>> @@ -871,16 +873,22 @@ static void ccb_apply_modify_hdlr(CcbUti
>>                                       sg->saAmfSGNumPrefStandbySUs =
>> *((SaUint32T *)value);
>>                               TRACE("Modified
>> saAmfSGNumPrefStandbySUs is '%u'", sg->saAmfSGNumPrefStandbySUs);
>>                       } else if (!strcmp(attribute->attrName,
>> "saAmfSGNumPrefInserviceSUs")) {
>> -                             if (value_is_deleted)
>> +                             if (value_is_deleted) {
>>                                       sg->saAmfSGNumPrefInserviceSUs =
>> sg_su_cnt(sg);
>> -                             else
>> +                                     sg-
>> >saAmfSGNumPrefInserviceSUs_configured = false;
>> +                             } else {
>>                                       sg->saAmfSGNumPrefInserviceSUs =
>> *((SaUint32T *)value);
>> +                                     sg-
>> >saAmfSGNumPrefInserviceSUs_configured = true;
>> +                             }
>>                               TRACE("Modified
>> saAmfSGNumPrefInserviceSUs is '%u'", sg->saAmfSGNumPrefInserviceSUs);
>>                       } else if (!strcmp(attribute->attrName,
>> "saAmfSGNumPrefAssignedSUs")) {
>> -                             if (value_is_deleted)
>> +                             if (value_is_deleted)  {
>>                                       sg->saAmfSGNumPrefAssignedSUs =
>> sg->saAmfSGNumPrefInserviceSUs;
>> -                             else
>> +                                     sg-
>> >saAmfSGNumPrefAssignedSUs_configured = false;
>> +                             } else {
>>                                       sg->saAmfSGNumPrefAssignedSUs =
>> *((SaUint32T *)value);
>> +                                     sg-
>> >saAmfSGNumPrefAssignedSUs_configured = true;
>> +                             }
>>                               TRACE("Modified
>> saAmfSGNumPrefAssignedSUs is '%u'", sg->saAmfSGNumPrefAssignedSUs);
>>                       } else if (!strcmp(attribute->attrName,
>> "saAmfSGMaxActiveSIsperSU")) {
>>                               if (value_is_deleted)
>> @@ -1415,6 +1423,36 @@ static void sg_ccb_apply_cb(CcbUtilOpera
>>       }
>>  }
>>
>> +/**
>> + * Adjusts saAmfSGNumPrefInserviceSUs and saAmfSGNumPrefAssignedSUs
>> when
>> + * for example an SU has been added or removed from the SG.
>> + * @param sg
>> + */
>> +static void adjust_num_preferred(AVD_SG *sg)
>> +{
>> +     /* adjust saAmfSGNumPrefInserviceSUs if not configured */
>> +     if (sg->saAmfSGNumPrefInserviceSUs_configured == false) {
>> +             sg->saAmfSGNumPrefInserviceSUs = sg_su_cnt(sg);
>> +             if ((sg->sg_type->saAmfSgtRedundancyModel ==
>> SA_AMF_2N_REDUNDANCY_MODEL) &&
>> +                             (sg->saAmfSGNumPrefInserviceSUs < 2)) {
>> +                     sg->saAmfSGNumPrefInserviceSUs = 2;
>> +             }
>> +             LOG_NO("'%s' saAmfSGNumPrefInserviceSUs adjusted to %u",
>> +                     sg->name.value, sg->saAmfSGNumPrefInserviceSUs);
>> +     }
>> +
>> +     /* adjust saAmfSGNumPrefAssignedSUs if not configured, only
>> applicable for
>> +      * the N-way and N-way active redundancy models
>> +      */
>> +     if ((sg->saAmfSGNumPrefAssignedSUs_configured == false) &&
>> +                     ((sg->sg_type->saAmfSgtRedundancyModel ==
>> SA_AMF_N_WAY_REDUNDANCY_MODEL) ||
>> +                             (sg->sg_type->saAmfSgtRedundancyModel ==
>> SA_AMF_N_WAY_ACTIVE_REDUNDANCY_MODEL))) {
>> +             sg->saAmfSGNumPrefAssignedSUs = sg-
>> >saAmfSGNumPrefInserviceSUs;
>> +             LOG_NO("'%s' saAmfSGNumPrefAssignedSUs adjusted to %u",
>> +                             sg->name.value, sg-
>> >saAmfSGNumPrefAssignedSUs);
>> +     }
>> +}
>> +
>>  void avd_sg_remove_su(AVD_SU* su)
>>  {
>>       AVD_SU *i_su = NULL;
>> @@ -1444,6 +1482,7 @@ void avd_sg_remove_su(AVD_SU* su)
>>               su->sg_of_su = NULL;
>>       } /* if (su->sg_of_su != AVD_SG_NULL) */
>>
>> +     adjust_num_preferred(sg);
>>       avd_verify_equal_ranked_su(sg);
>>       return;
>>  }
>> @@ -1470,6 +1509,8 @@ void avd_sg_add_su(AVD_SU* su)
>>               prev_su->sg_list_su_next = su;
>>               su->sg_list_su_next = i_su;
>>       }
>> +
>> +     adjust_num_preferred(su->sg_of_su);
>>       avd_verify_equal_ranked_su(su->sg_of_su);
>>  }
>>
>> @@ -1563,32 +1604,9 @@ void avd_sg_adjust_config(AVD_SG *sg)
>>                               su_nd_attribute_update(su,
>> saAmfSUFailOver_ID);
>>                       }
>>               }
>> -
>> -     }
>> -
>> -     /* adjust saAmfSGNumPrefInserviceSUs if not configured */
>> -     if (sg->saAmfSGNumPrefInserviceSUs == 0) {
>> -             sg->saAmfSGNumPrefInserviceSUs = sg_su_cnt(sg);
>> -             if ((sg->sg_type->saAmfSgtRedundancyModel ==
>> SA_AMF_2N_REDUNDANCY_MODEL) &&
>> -                             (sg->saAmfSGNumPrefInserviceSUs < 2)) {
>> -                     sg->saAmfSGNumPrefInserviceSUs = 2;
>> -                     LOG_NO("'%s' saAmfSGNumPrefInserviceSUs adjusted
>> to 2", sg->name.value);
>> -             }
>> -     }
>> -
>> -     /* adjust saAmfSGNumPrefAssignedSUs if not configured, only
>> applicable for
>> -      * the N-way and N-way active redundancy models
>> -      */
>> -     if ((sg->saAmfSGNumPrefAssignedSUs == 0) &&
>> -                     ((sg->sg_type->saAmfSgtRedundancyModel ==
>> SA_AMF_N_WAY_REDUNDANCY_MODEL) ||
>> -                             (sg->sg_type->saAmfSgtRedundancyModel ==
>> SA_AMF_N_WAY_ACTIVE_REDUNDANCY_MODEL))) {
>> -             sg->saAmfSGNumPrefAssignedSUs = sg-
>> >saAmfSGNumPrefInserviceSUs;
>> -             LOG_NO("'%s' saAmfSGNumPrefAssignedSUs adjusted to %u",
>> -                             sg->name.value, sg-
>> >saAmfSGNumPrefAssignedSUs);
>>       }
>>  }
>>
>> -
>>  /**
>>   * @brief Counts number of instantiated su in the sg.
>>   * @param Service Group
>>
>> ------------------------------------------------------------------------------
>> Learn Graph Databases - Download FREE O'Reilly Book
>> "Graph Databases" is the definitive new guide to graph databases and their
>> applications. Written by three acclaimed leaders in the field,
>> this first edition is now available. Download your free book today!
>> http://p.sf.net/sfu/13534_NeoTech
>> _______________________________________________
>> Opensaf-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>
> ------------------------------------------------------------------------------
> Learn Graph Databases - Download FREE O'Reilly Book
> "Graph Databases" is the definitive new guide to graph databases and their
> applications. Written by three acclaimed leaders in the field,
> this first edition is now available. Download your free book today!
> http://p.sf.net/sfu/NeoTech
> _______________________________________________
> Opensaf-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel

------------------------------------------------------------------------------
Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to