Hi Hoa,
One comment inline.
Thanks
Minh
On 20/06/18 13:38, Hoa Le wrote:
> Currently, when updating the last event info, mdstest identify services
> using their svc_id. This will cause confusion when several services was
> installed with the same svc_id (on different mds_dest-s). If a service
> subscribes to this svc_id, the service will retrieve several event info
> with the same svc_id. When storing these event info to svcevt array, the
> info are overwritten one by one and only the last info will be stored.
>
> This patch helps avoid the above situation by identifying these
> services using both their svc_id and mds_dest. This helps the event
> info, from different service, be separatedly stored to svcevt array.
> subscr_count value will also be updated in accordance with these
> event info.
> ---
> src/mds/apitest/mdstipc_api.c | 13 ++--
> src/mds/apitest/mdstipc_conf.c | 162
> +++++++++++++++++++++++++++++++++++++----
> 2 files changed, 152 insertions(+), 23 deletions(-)
>
> diff --git a/src/mds/apitest/mdstipc_api.c b/src/mds/apitest/mdstipc_api.c
> index 22a4386..090af99 100644
> --- a/src/mds/apitest/mdstipc_api.c
> +++ b/src/mds/apitest/mdstipc_api.c
> @@ -1090,7 +1090,7 @@ static int tet_verify_version(MDS_HDL mds_hdl,
> NCSMDS_SVC_ID your_scv_id,
> MDS_SVC_PVT_SUB_PART_VER svc_pvt_ver,
> NCSMDS_CHG change)
> {
> - int i, j, k, ret_val = 0;
> + int i, j, k;
> if (is_service_on_adest(mds_hdl, your_scv_id) == NCSCC_RC_SUCCESS) {
> for (i = 0; i < gl_tet_adest.svc_count; i++) {
> if (gl_tet_adest.svc[i].svc_id == your_scv_id) {
> @@ -1107,9 +1107,8 @@ static int tet_verify_version(MDS_HDL mds_hdl,
> NCSMDS_SVC_ID your_scv_id,
> .svcevt[j]
> .rem_svc_pvt_ver ==
> svc_pvt_ver))
> - ret_val = 1;
> - else
> - ret_val = 0;
> + return 1;
> +
> }
> }
> }
> @@ -1137,16 +1136,14 @@ static int tet_verify_version(MDS_HDL mds_hdl,
> NCSMDS_SVC_ID your_scv_id,
> .svcevt[k]
>
> .rem_svc_pvt_ver ==
> svc_pvt_ver))
> - ret_val = 1;
> - else
> - ret_val = 0;
> + return 1;
> }
> }
> }
> }
> }
> }
> - return ret_val;
> + return 0;
> }
> void tet_svc_subscr_VDEST_1()
> {
> diff --git a/src/mds/apitest/mdstipc_conf.c b/src/mds/apitest/mdstipc_conf.c
> index bf4c1de..61bf27f 100644
> --- a/src/mds/apitest/mdstipc_conf.c
> +++ b/src/mds/apitest/mdstipc_conf.c
> @@ -557,7 +557,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> NCSMDS_SCOPE_TYPE scope, uint8_t num_svcs,
> MDS_SVC_ID *svc_ids)
> {
> - int i, j, k, l, FOUND;
> + int i, j, l, FOUND;
> uint32_t YES_ADEST;
> NCSMDS_INFO svc_to_mds_info;
> /*Find whether this Service is on Adest or Vdest*/
> @@ -594,6 +594,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svc[j]
> .subscr.svc_ids = svc_ids;
>
> /*gl_tet_vdest[i].svc[j].subscr.evt_map=*/
> + /* Fix for ticket #2798
> gl_tet_vdest[i]
> .svc[j]
> .subscr_count += num_svcs;
> @@ -611,6 +612,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svcevt[k]
> .ur_svc_id = svc_id;
> }
> + */
> FOUND = 1;
> break;
> }
> @@ -642,6 +644,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svc_ids =
> svc_ids;
>
> /*gl_tet_vdest[i].svc[j].subscr.evt_map=*/
> + /* Ticket #2798
> gl_tet_vdest[i]
> .svc[j]
>
> .subscr_count +=
> @@ -667,6 +670,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
>
> .ur_svc_id =
>
> svc_id;
> }
> + */
> FOUND = 1;
> break;
> }
> @@ -688,6 +692,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> scope;
> gl_tet_adest.svc[i].subscr.svc_ids =
> svc_ids;
> + /* Fix for ticket #2798
> gl_tet_adest.svc[i].subscr_count +=
> num_svcs;
> for (k = 0; k < num_svcs; k++) {
> @@ -700,6 +705,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svcevt[k]
> .ur_svc_id = svc_id;
> }
> + */
> break;
> }
> if ((gl_tet_adest.svc[i].svc_id == svc_id) &&
> @@ -713,8 +719,10 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .subscr.scope = scope;
> gl_tet_adest.svc[i]
> .subscr.svc_ids = svc_ids;
> + /* Fix for ticket #2798
> gl_tet_adest.svc[i]
> .subscr_count += num_svcs;
> +
> for (k = 0; k < num_svcs; k++) {
> gl_tet_adest.svc[i]
> .svcevt[k]
> @@ -726,6 +734,7 @@ uint32_t mds_service_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svcevt[k]
> .ur_svc_id = svc_id;
> }
> + */
> break;
> }
> }
> @@ -743,7 +752,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> NCSMDS_SCOPE_TYPE scope,
> uint8_t num_svcs, MDS_SVC_ID *svc_ids)
> {
> - int i, j, k, l, FOUND;
> + int i, j, l, FOUND;
> uint32_t YES_ADEST;
> NCSMDS_INFO svc_to_mds_info;
> /*Find whether this Service is on Adest or Vdest*/
> @@ -780,6 +789,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svc[j]
> .subscr.svc_ids = svc_ids;
>
> /*gl_tet_vdest[i].svc[j].subscr.evt_map=*/
> + /* Fix for ticket #2798
> gl_tet_vdest[i]
> .svc[j]
> .subscr_count += num_svcs;
> @@ -797,6 +807,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svcevt[k]
> .ur_svc_id = svc_id;
> }
> + */
> FOUND = 1;
> break;
> }
> @@ -828,6 +839,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svc_ids =
> svc_ids;
>
> /*gl_tet_vdest[i].svc[j].subscr.evt_map=*/
> + /* Ticket #2798
> gl_tet_vdest[i]
> .svc[j]
>
> .subscr_count +=
> @@ -853,6 +865,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
>
> .ur_svc_id =
>
> svc_id;
> }
> + */
> FOUND = 1;
> break;
> }
> @@ -874,6 +887,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> scope;
> gl_tet_adest.svc[i].subscr.svc_ids =
> svc_ids;
> + /* Fix for ticket #2798
> gl_tet_adest.svc[i].subscr_count +=
> num_svcs;
> for (k = 0; k < num_svcs; k++) {
> @@ -886,6 +900,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svcevt[k]
> .ur_svc_id = svc_id;
> }
> + */
> break;
> }
> if ((gl_tet_adest.svc[i].svc_id == svc_id) &&
> @@ -899,6 +914,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .subscr.scope = scope;
> gl_tet_adest.svc[i]
> .subscr.svc_ids = svc_ids;
> + /* Fix for ticket #2798
> gl_tet_adest.svc[i]
> .subscr_count += num_svcs;
> for (k = 0; k < num_svcs; k++) {
> @@ -912,6 +928,7 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL mds_hdl,
> MDS_SVC_ID svc_id,
> .svcevt[k]
> .ur_svc_id = svc_id;
> }
> + */
> break;
> }
> }
> @@ -925,6 +942,52 @@ uint32_t mds_service_redundant_subscribe(MDS_HDL
> mds_hdl, MDS_SVC_ID svc_id,
> return NCSCC_RC_FAILURE;
> }
> }
> +/******************************************************************************
> +
> + Function NAME: tet_remove_subscribed_events
> +
> + DESCRIPTION: This routine removes the subscribed events from svcevt array
> + when a service requested MDS to cancel subscription for the service events
> + of the given service IDs.
> +
> + ARGUMENTS:
> + num_svcs: Number of service-identifiers being unsubscribed
> + svc_ids: The list (array) of service-identifiers being unsubscribed
> to
> + subscr_count: Number of subscribed services event info.
> + svc_evts: The list (array) of subscribed services event info.
> +
> +******************************************************************************/
> +
> +void tet_remove_subscribed_events(int num_svcs, MDS_SVC_ID *svc_ids,
> + int *subscr_count,
> + TET_EVENT_INFO *svc_evts)
> +{
> + int curr_index = 0;
> + int new_index = 0;
> +
> + int current_count = *subscr_count;
> +
> + pthread_mutex_lock(&gl_mutex);
> + while (curr_index < current_count) {
> + bool deleted = false;
> + for (int i = 0; i < num_svcs; i++) {
> + if (svc_evts[curr_index].svc_id == svc_ids[i]){
> + *subscr_count -= 1;
> + deleted = true;
> + break;
> + }
> + }
> +
> + if (!deleted){
> + if (curr_index > new_index){
> + svc_evts[new_index] = svc_evts[curr_index];
[Minh]: If I don't misunderstand, this line is shrinking the svc_evts
array? If so, the svc_evts[new_index] and svc_evts[curr_index] are both
to be removed. And the below "new_index += 1" can be stayed at
not-to-be-removed item in svc_evts
> + }
> + new_index += 1;
> + }
> + curr_index += 1;
> + }
> + pthread_mutex_unlock(&gl_mutex);
> +}
>
> uint32_t mds_service_cancel_subscription(MDS_HDL mds_hdl, MDS_SVC_ID svc_id,
> uint8_t num_svcs, MDS_SVC_ID *svc_ids)
> @@ -940,14 +1003,18 @@ uint32_t mds_service_cancel_subscription(MDS_HDL
> mds_hdl, MDS_SVC_ID svc_id,
>
> if (ncsmds_api(&svc_to_mds_info) == NCSCC_RC_SUCCESS) {
> printf("\n MDS CANCEL SUBSCRIBE is SUCCESSFULL");
> - pthread_mutex_lock(&gl_mutex);
> + // Decrease subscr_count and remove subscribed services events.
> FOUND = 0;
> for (i = 0; i < gl_vdest_indx; i++) {
> for (j = 0; j < gl_tet_vdest[i].svc_count; j++) {
> if (gl_tet_vdest[i].svc[j].svc_id == svc_id &&
> gl_tet_vdest[i].mds_pwe1_hdl == mds_hdl) {
> - gl_tet_vdest[i].svc[j].subscr_count -=
> - num_svcs;
> + tet_remove_subscribed_events(
> + num_svcs, svc_ids,
> + &gl_tet_vdest[i].
> + svc[j].subscr_count,
> + gl_tet_vdest[i].
> + svc[j].svcevt);
> FOUND = 1;
> break;
> }
> @@ -960,10 +1027,11 @@ uint32_t mds_service_cancel_subscription(MDS_HDL
> mds_hdl, MDS_SVC_ID svc_id,
> .pwe[k]
> .mds_pwe_hdl ==
> mds_hdl) {
> - gl_tet_vdest[i]
> - .svc[j]
> - .subscr_count -=
> - num_svcs;
> +
> tet_remove_subscribed_events(
> +
> num_svcs,
> + svc_ids,
> +
> &gl_tet_vdest[i].svc[j].subscr_count,
> +
> gl_tet_vdest[i].svc[j].svcevt);
> FOUND = 1;
> break;
> }
> @@ -976,12 +1044,16 @@ uint32_t mds_service_cancel_subscription(MDS_HDL
> mds_hdl, MDS_SVC_ID svc_id,
> if (!FOUND) {
> for (i = 0; i < gl_tet_adest.svc_count; i++)
> if (gl_tet_adest.svc[i].svc_id == svc_id) {
> - gl_tet_adest.svc[i].subscr_count -=
> - num_svcs;
> + tet_remove_subscribed_events(
> + num_svcs,
> + svc_ids,
> + &gl_tet_adest.svc[i].
> + subscr_count,
> + gl_tet_adest.svc[i].
> + svcevt);
> break;
> }
> }
> - pthread_mutex_unlock(&gl_mutex);
> return NCSCC_RC_SUCCESS;
> } else {
> printf(
> @@ -2100,7 +2172,7 @@ uint32_t tet_mds_cb_direct_rcv(NCSMDS_CALLBACK_INFO
> *mds_to_svc_info)
>
> uint32_t tet_mds_svc_event(NCSMDS_CALLBACK_INFO *mds_to_svc_info)
> {
> - int i, j, k;
> + int i, j, k, FOUND;
> TET_EVENT_INFO gl_event_data;
>
> gl_event_data.ur_svc_id = mds_to_svc_info->info.svc_evt.i_your_id;
> @@ -2115,6 +2187,8 @@ uint32_t tet_mds_svc_event(NCSMDS_CALLBACK_INFO
> *mds_to_svc_info)
> gl_event_data.rem_svc_pvt_ver =
> mds_to_svc_info->info.svc_evt.i_rem_svc_pvt_ver;
> gl_event_data.svc_pwe_hdl = mds_to_svc_info->info.svc_evt.svc_pwe_hdl;
> +
> + FOUND = 0;
> pthread_mutex_lock(&gl_mutex);
> if (is_service_on_adest(gl_event_data.svc_pwe_hdl,
> gl_event_data.ur_svc_id) == NCSCC_RC_SUCCESS) {
> @@ -2124,15 +2198,40 @@ uint32_t tet_mds_svc_event(NCSMDS_CALLBACK_INFO
> *mds_to_svc_info)
> /*find to which service this EVENT came*/
> if (gl_tet_adest.svc[i].svc_id ==
> gl_event_data.ur_svc_id) {
> + /* update the last EVENT of subscribed svc */
> for (j = 0;
> j < gl_tet_adest.svc[i].subscr_count;
> j++) {
> if (gl_tet_adest.svc[i]
> .svcevt[j]
> - .svc_id == gl_event_data.svc_id)
> + .svc_id == gl_event_data.svc_id
> + && gl_tet_adest.svc[i].
> + svcevt[j].
> + dest == gl_event_data.dest) {
> gl_tet_adest.svc[i].svcevt[j] =
> gl_event_data;
> + FOUND = 1;
> + break;
> + }
> + }
> +
> + if (!FOUND && gl_tet_adest.svc[i].
> + subscr_count < 100) {
> + /*TODO: Update to support more than 100
> + * subscribed services.
> + * svcevt array currently only stores
> + * the "last state" events of maximum
> + * 100 first subscribed services. If
> + * the event, which has just come, is
> + * the 101st svc, it'll not be stored.
> + */
> + int tmp = gl_tet_adest.svc[i].
> + subscr_count;
> + gl_tet_adest.svc[i].subscr_count += 1;
> + gl_tet_adest.svc[i].svcevt[tmp] =
> + gl_event_data;
> }
> + break;
> }
> }
> } else {
> @@ -2142,6 +2241,8 @@ uint32_t tet_mds_svc_event(NCSMDS_CALLBACK_INFO
> *mds_to_svc_info)
> for (j = 0; j < gl_tet_vdest[i].svc_count; j++) {
> if (gl_tet_vdest[i].svc[j].svc_id ==
> gl_event_data.ur_svc_id) {
> + /* update the last EVENT of subscribed
> + * svc */
> for (k = 0; k < gl_tet_vdest[i]
> .svc[j]
> .subscr_count;
> @@ -2150,14 +2251,45 @@ uint32_t tet_mds_svc_event(NCSMDS_CALLBACK_INFO
> *mds_to_svc_info)
> .svc[j]
> .svcevt[k]
> .svc_id ==
> - gl_event_data.svc_id)
> + gl_event_data.svc_id &&
> + gl_tet_vdest[i].svc[k].
> + svcevt[k].
> + dest == gl_event_data.
> + dest) {
> gl_tet_vdest[i]
> .svc[j]
> .svcevt[k] =
> gl_event_data;
> + FOUND = 1;
> + break;
> + }
> }
> + if (!FOUND && gl_tet_vdest[i].svc[j].
> + subscr_count < 100) {
> + /*TODO: Update to support more
> + * than 100 subscribed svcs.
> + * svcevt array currently only
> + * stores the "last state"
> + * events of maximum 100 first
> + * subscribed services. If the
> + * event, which has just come,
> + * is the 101st service, it'll
> + * not be stored.
> + */
> + int tmp = gl_tet_vdest[i].
> + svc[j].
> + subscr_count;
> + gl_tet_vdest[i].svc[j].
> + subscr_count += 1;
> + gl_tet_vdest[i].svc[j].
> + svcevt[tmp] = gl_event_data;
> + }
> + FOUND = 1;
> + break;
> }
> }
> + if (FOUND)
> + break;
> }
> }
> pthread_mutex_unlock(&gl_mutex);
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
