> -----Original Message-----
> From: Anders Bjornerstedt [mailto:anders.bjornerst...@ericsson.com]
> Sent: den 10 september 2014 11:37
> To: Hans Feldt
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: Re: [devel] [PATCH 2 of 2] imma: use mds_auth_server_disconnect
> [#1050]
>
> A few questions below, see inline.
>
> Hans Feldt wrote:
> > osaf/libs/agents/saf/imma/imma.h | 1 +
> > osaf/libs/agents/saf/imma/imma_init.c | 6 ++++++
> > osaf/libs/agents/saf/imma/imma_mds.c | 7 ++++---
> > 3 files changed, 11 insertions(+), 3 deletions(-)
> >
> >
> > diff --git a/osaf/libs/agents/saf/imma/imma.h
> > b/osaf/libs/agents/saf/imma/imma.h
> > --- a/osaf/libs/agents/saf/imma/imma.h
> > +++ b/osaf/libs/agents/saf/imma/imma.h
> > @@ -36,6 +36,7 @@
> > #include "imma_mds.h"
> >
> > extern IMMA_CB imma_cb;
> > +extern const char *imma_sockname;
> >
> > unsigned int imma_shutdown(NCSMDS_SVC_ID sv_id);
> > unsigned int imma_startup(NCSMDS_SVC_ID sv_id);
> > diff --git a/osaf/libs/agents/saf/imma/imma_init.c
> > b/osaf/libs/agents/saf/imma/imma_init.c
> > --- a/osaf/libs/agents/saf/imma/imma_init.c
> > +++ b/osaf/libs/agents/saf/imma/imma_init.c
> > @@ -30,6 +30,7 @@
> > #include "osaf_poll.h"
> > #include "osaf_extended_name.h"
> > #include "saAis.h"
> > +#include "mds_dl_api.h"
> >
> >
> > /*****************************************************************************
> > global data used by IMMA
> > @@ -207,6 +208,11 @@ static uint32_t imma_destroy(NCSMDS_SVC_
> > IMMA_CB *cb = &imma_cb;
> > TRACE_ENTER();
> >
> > + if (mds_auth_server_disconnect(imma_sockname,
> > + cb->imma_mds_adest, 10000) != NCSCC_RC_SUCCESS) {
> > + LOG_WA("%s: mds_auth_server_disconnect failed", __FUNCTION__);
> >
> (1)
> Here a warning is printed if mds_auth_server_disconnect() fails.
> But besides the warning execution continues as if this is no serious
> problem.
> Without going into why this could happen, we need to know the potential
> effects if it did happen.
> In other words, is this a self correcting problem ?
[Hans] on the imma side the library is getting "reset" here so it will clear
all state. On the server/immnd side the new MDS timeout will kick in and clean
out the info related to this particular MDS core. In the normal case this
timeout does nothing since the explicit new disconnect has done the job.
> > + }
> > +
> > /* MDS unregister. */
> > imma_mds_unregister(cb);
> >
> > diff --git a/osaf/libs/agents/saf/imma/imma_mds.c
> > b/osaf/libs/agents/saf/imma/imma_mds.c
> > --- a/osaf/libs/agents/saf/imma/imma_mds.c
> > +++ b/osaf/libs/agents/saf/imma/imma_mds.c
> > @@ -29,7 +29,7 @@
> > #include "ncs_util.h"
> > #include "mds_dl_api.h"
> >
> > -static const char *sockname = PKGLOCALSTATEDIR "/immnd.sock";
> > +const char *imma_sockname = PKGLOCALSTATEDIR "/immnd.sock";
> >
> > static uint32_t imma_mds_enc_flat(IMMA_CB *cb, MDS_CALLBACK_ENC_FLAT_INFO
> > *info);
> > static uint32_t imma_mds_dec_flat(IMMA_CB *cb, MDS_CALLBACK_DEC_FLAT_INFO
> > *info);
> > @@ -420,16 +420,17 @@ static uint32_t imma_mds_svc_evt(IMMA_CB
> > case NCSMDS_UP:
> > TRACE_3("IMMND UP");
> >
> > m_NCS_LOCK(&cb->immnd_sync_lock,NCS_LOCK_WRITE);/*special sync lock*/
> > - cb->is_immnd_up = true;
> >
> (2)
> Why do you move the above line ^^^^^^ down to ...
> > cb->immnd_mds_dest = svc_evt->i_dest;
> >
> > /* (Re-)connect and register our MDS dest with auth
> > server in immnd */
> > - if (mds_auth_server_connect(sockname,
> > + if (mds_auth_server_connect(imma_sockname,
> > cb->imma_mds_adest, 10000) !=
> > NCSCC_RC_SUCCESS) {
> > /* server UP indication yet this does not
> > work... */
> > LOG_WA("%s: mds_auth_server_connect failed",
> > __FUNCTION__);
> > }
> >
> (2 continued)
> ... here below vvvv when you will end up here unconditionally anyway?
[Hans] should have commented that...
is_immnd_up is checked in so many places and I don't want that logic to proceed
without having done the registration with the server. A race condition might
happen.
> It only makes sense if mds_auth_server_connect() could depend on
> cb->is_immnd_up.
> But mds_auth_server_connect is a generic libary function so it can not
> depend on the imma cb.
>
> (3)
> Also, question (1) above can be asked about the warning here above.
> What would be the consequences if the mds_auth_server_connect() fails
> when we still just continue.
> Is the problem self correcting ?
[Hans] no in that case initialize would fail with the "MDS problem" log.
>
> /AndersBj
> >
> > + cb->is_immnd_up = true;
> > +
> > if (cb->immnd_sync_awaited == true)
> > m_NCS_SEL_OBJ_IND(&cb->immnd_sync_sel);
> >
> > m_NCS_UNLOCK(&cb->immnd_sync_lock,NCS_LOCK_WRITE);/*special sync lock*/
> >
>
>
> ------------------------------------------------------------------------------
> 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
> Opensaf-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
