Thx Minh,
When it gets ACKs from others, please help to merge.
In the mean time, I will update the MR with pro/cons. This kind of discussion
will also go to read me file.
Cheers,
Thanh
-----Original Message-----
From: Minh Hon Chau <minh.c...@dektech.com.au>
Sent: Wednesday, 15 July 2020 10:29 PM
To: Thanh Nguyen <thanh.ngu...@dektech.com.au>; Thang Duc Nguyen
<thang.d.ngu...@dektech.com.au>; Thuan Tran <thuan.t...@dektech.com.au>
Cc: opensaf-devel@lists.sourceforge.net
Subject: Re: [PATCH 1/1] ntf: Handle IMM multiple value attribute in
notification [#3200]
Hi aThanh,
Ack from me.
I guess we will document the README with this change to mention the pros/cons
of either not to fill any old values or just to fill some of them, so that the
reason of this change can be clarified?
Thanks
Minh
On 14/7/20 10:57 am, Thanh Nguyen wrote:
> When IMM changes an attribute with NOTIFY flag, ntfimcn currently
> sends the changed attribute values in notification data.
> ntfimcn also fetches the old attribute values, corresponding to the
> changed attribute values, if exist in IMM, and include the old
> attribute values in notification data.
>
> 1) With this enahanement, for IMM atrribute with multiple values, if
> the number of old attribute values is greater than the number of new
> attribute values, the old attribute values will not be published in
> the notification data
> 2) ntftest suite is update
> ---
> src/ntf/apitest/test_ntf_imcn.cc | 72 +++++++++++------------------
> src/ntf/ntfimcnd/ntfimcn_notifier.c | 13 +++++-
> 2 files changed, 38 insertions(+), 47 deletions(-)
>
> diff --git a/src/ntf/apitest/test_ntf_imcn.cc
> b/src/ntf/apitest/test_ntf_imcn.cc
> index 0443491a9..082f69c46 100644
> --- a/src/ntf/apitest/test_ntf_imcn.cc
> +++ b/src/ntf/apitest/test_ntf_imcn.cc
> @@ -61,6 +61,19 @@ static char BUF3_STR[sizeof(BUF3) + 1] = { '\0' };
>
> #define OLD_ATTR_PRESENT_DEFAULT SA_FALSE
>
> +/*
> + * Ticket-3200
> + * For IMM attributes with multiple values, if the number old
> +attribute
> + * values is greater than the number of new attribute values the old
> +values
> + * are not included in the notification data
> + * The following test cases are changed, not to expect old attribute
> +values
> + * objectModifyTest_33
> + * objectModifyTest_34
> + * objectModifyTest_36
> + * objectMultiCcbTest_38
> + * objectMultiCcbTest_39
> + *
> + */
> /**
> * Callback routine, called when subscribed notification arrives.
> */
> @@ -4400,9 +4413,7 @@ void objectModifyTest_33(void) {
>
> /* modify an object */
> SaStringT svar = STRINGVAR3;
> - SaStringT svarOld = STRINGVAR1;
> SaDoubleT dvar = DOUBLEVAR3;
> - SaDoubleT dvarOld = DOUBLEVAR1;
> snprintf(command, MAX_DATA,
> "immcfg -t 20 -a testStringCfg=%s -a testDoubleCfg=%lf %s",
> svar, dvar, DNTESTCFG);
> @@ -4444,12 +4455,9 @@ void objectModifyTest_33(void) {
> SaUint32T ccbLast = 1;
> safassert(set_attr_change_scalar(&n_exp, 3, 3,
> SA_NTF_VALUE_UINT32, &ccbLast), SA_AIS_OK);
> - safassert(set_attr_change_str(&n_exp, 4, 4, svar,
> - SA_TRUE, svarOld), SA_AIS_OK);
> + safassert(set_attr_change_str(&n_exp, 4, 4, svar), SA_AIS_OK);
> safassert(set_attr_change_scalar(&n_exp, 5, 5,
> - SA_NTF_VALUE_DOUBLE, &dvar, SA_TRUE, &dvarOld),
> - SA_AIS_OK);
> -
> + SA_NTF_VALUE_DOUBLE, &dvar), SA_AIS_OK);
> if (!compare_notifs(&n_exp, &rec_notif_data)) {
> print_notif(&n_exp);
> print_notif(&rec_notif_data);
> @@ -4478,7 +4486,6 @@ void objectModifyTest_34(void) {
>
> /* modify an object */
> SaTimeT tvar = TIMEVAR3;
> - SaTimeT tvarOld = TIMEVAR1;
> snprintf(command, MAX_DATA, "immcfg -t 20 -a testTimeCfg=%lld %s",
> (SaInt64T)tvar, DNTESTCFG);
> assert(system(command) != -1);
> @@ -4517,8 +4524,7 @@ void objectModifyTest_34(void) {
> safassert(set_attr_change_scalar(&n_exp, 3, 3,
> SA_NTF_VALUE_UINT32, &ccbLast), SA_AIS_OK);
> safassert(set_attr_change_scalar(&n_exp, 4, 4,
> - SA_NTF_VALUE_INT64, &tvar, SA_TRUE, &tvarOld),
> - SA_AIS_OK);
> + SA_NTF_VALUE_INT64, &tvar), SA_AIS_OK);
>
> if (!compare_notifs(&n_exp, &rec_notif_data)) {
> print_notif(&n_exp);
> @@ -4680,14 +4686,11 @@ void objectModifyTest_36(void) {
> SaUint32T ccbLast = 1;
> safassert(set_attr_change_scalar(&n_exp, 3, 3,
> SA_NTF_VALUE_UINT32, &ccbLast), SA_AIS_OK);
> - safassert(set_attr_change_str(&n_exp, 4, 4, soldvar, SA_TRUE, soldvar),
> - SA_AIS_OK);
> + safassert(set_attr_change_str(&n_exp, 4, 4, soldvar), SA_AIS_OK);
> safassert(set_attr_change_scalar(&n_exp, 5, 5,
> - SA_NTF_VALUE_INT32, &ivar2, SA_TRUE, &idelvar),
> - SA_AIS_OK);
> + SA_NTF_VALUE_INT32, &ivar2), SA_AIS_OK);
> safassert(set_attr_change_scalar(&n_exp, 6, 5,
> - SA_NTF_VALUE_INT32, &ivar1, SA_TRUE, &ivar1),
> - SA_AIS_OK);
> + SA_NTF_VALUE_INT32, &ivar1), SA_AIS_OK);
>
> if (!compare_notifs(&n_exp, &rec_notif_data)) {
> print_notif(&n_exp);
> @@ -4917,12 +4920,8 @@ void objectMultiCcbTest_38(void) {
> /* modify an object */
> SaUint32T var1 = 32000;
> SaInt32T var2 = -32000;
> - SaUint32T var1old = UINT32VAR2;
> - SaInt32T var2old = INT32VAR1;
> SaUint64T var3 = 64000;
> SaInt64T var4 = -64000;
> - SaUint64T var3old = UINT64VAR1;
> - SaInt64T var4old = INT64VAR1;
> SaImmHandleT omHandle = 0;
> SaImmAdminOwnerHandleT ownerHandle = 0;
> SaImmCcbHandleT immCcbHandle = 0;
> @@ -5034,15 +5033,10 @@ void objectMultiCcbTest_38(void) {
> safassert(set_attr_change_scalar(&n_exp, 3, 3,
> SA_NTF_VALUE_UINT32,
> &ccbLast), SA_AIS_OK);
> -
> safassert(set_attr_change_scalar(
> - &n_exp, 4, 4, SA_NTF_VALUE_UINT32, &var1, SA_TRUE,
> - reinterpret_cast<void*>(&var1old)),
> - SA_AIS_OK);
> - safassert(set_attr_change_scalar(
> - &n_exp, 5, 5, SA_NTF_VALUE_INT32, &var2, SA_TRUE,
> - reinterpret_cast<void*>(&var2old)),
> - SA_AIS_OK);
> + &n_exp, 4, 4, SA_NTF_VALUE_UINT32, &var1), SA_AIS_OK);
> + safassert(set_attr_change_scalar(
> + &n_exp, 5, 5, SA_NTF_VALUE_INT32, &var2), SA_AIS_OK);
> } else if (rec_notif_data.populated && noOfNotifs == 1) {
> safassert(set_ntf(&n_exp, SA_NTF_ATTRIBUTE_CHANGED,
> DNTESTCFG, 6, 6),
> @@ -5075,15 +5069,9 @@ void objectMultiCcbTest_38(void) {
> SA_NTF_VALUE_UINT32,
> &ccbLast), SA_AIS_OK);
> safassert(set_attr_change_scalar(
> - &n_exp, 4, 4, SA_NTF_VALUE_UINT64, &var3,
> - SA_TRUE,
> - reinterpret_cast<void*>(&var3old)),
> - SA_AIS_OK);
> + &n_exp, 4, 4, SA_NTF_VALUE_UINT64, &var3), SA_AIS_OK);
> safassert(set_attr_change_scalar(
> - &n_exp, 5, 5, SA_NTF_VALUE_INT64, &var4,
> - SA_TRUE,
> - reinterpret_cast<void*>(&var4old)),
> - SA_AIS_OK);
> + &n_exp, 5, 5, SA_NTF_VALUE_INT64, &var4), SA_AIS_OK);
> } else {
> error = SA_AIS_ERR_FAILED_OPERATION;
> break;
> @@ -5345,17 +5333,11 @@ void objectMultiCcbTest_39(void) {
> SA_NTF_VALUE_UINT32,
> &ccbLast), SA_AIS_OK);
> safassert(set_attr_change_scalar(&n_exp, 4, 6,
> - SA_NTF_VALUE_FLOAT,
> - &oldvar6, SA_TRUE, &var6),
> - SA_AIS_OK);
> + SA_NTF_VALUE_FLOAT, &oldvar6), SA_AIS_OK);
> safassert(set_attr_change_scalar(&n_exp, 5, 6,
> - SA_NTF_VALUE_FLOAT,
> - &oldvar66, SA_TRUE, &oldvar66),
> - SA_AIS_OK);
> + SA_NTF_VALUE_FLOAT, &oldvar66), SA_AIS_OK);
> safassert(set_attr_change_scalar(&n_exp, 6, 7,
> - SA_NTF_VALUE_DOUBLE,
> - &oldvar5, SA_TRUE, &oldvar5),
> - SA_AIS_OK);
> + SA_NTF_VALUE_DOUBLE, &oldvar5), SA_AIS_OK);
>
> } else {
> error = SA_AIS_ERR_FAILED_OPERATION; diff --git
> a/src/ntf/ntfimcnd/ntfimcn_notifier.c
> b/src/ntf/ntfimcnd/ntfimcn_notifier.c
> index e3dbe0639..a4509c546 100644
> --- a/src/ntf/ntfimcnd/ntfimcn_notifier.c
> +++ b/src/ntf/ntfimcnd/ntfimcn_notifier.c
> @@ -876,8 +876,17 @@ static int fill_attribute_info_modify(
> __FUNCTION__);
> goto done;
> }
> - // There can be more new attribute values than
> - // old attribute values
> + // No need to populate notification data when
> + // the number of old attributes is grater than
> + // the number of new attributes
> + if (old_imm_attr_value &&
> + (old_imm_attr_value
> + ->attrValuesNumber >
> + imm_attr_mods
> + ->modAttr.attrValuesNumber))
> + continue;
> + // The number of old attributes is less than or
> + // equal to the number of new attribute values
> if (old_imm_attr_value &&
> (vindex < old_imm_attr_value
> ->attrValuesNumber)) {
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
