Hi Zoran,
Here's Anders' comment on the previous patches when I tried to handle
the new message types in IMM service.
> I would like to avoid the solution below of re-coding fevs messages to
> the old message type in fevs_local_checks.
>
> Several solutions are possible but one thing to remember here is that
> we are in this case not trying to
> discriminate between any new or old functionality. Thus here it is not
> necessary to have quick
> reactivity to protocol46Allowed for these two cases:
>
> * aug-init from inside OI callback
> * reply from delete-callback
>
> That is, it is no big problem if some users continue to use the old
> SaNameT based message protocols for
> these two message types to the server, indefinitely.
>
> I suggest adding one new member to IMMSV_ND2A_INIT_RSP in immsv_evt.h:
>
> typedef struct immsv_nd2a_init_rsp {
> SaImmHandleT immHandle;
> SaAisErrorT error;
> SaVersionT version; <========== new member
> } IMMSV_ND2A_INIT_RSP;
>
> This struct is replied on the request for OM or OI handle initialize.
>
> You would implement encode/(decode for the new version member.
> But you would for now only need to set to A.2.15 on OI initialize if
> and only if protocol46 is allowed
> in the server when the request arrives in the server.
>
> This means that all OI handles that are initialized before protocol 46
> is allowed would get it set to _.0.0
> or to A.2.14. The important thing is that it is set to A.2.15 only
> after/when an initialize is done after
> protocol46 is allowed on the server and not set to this value before that.
>
> This value (or just a bool) needs to be stored in the client_node
> struct when the reply from oi-initialize is received,
> if the error code is OK.
>
> I suggest adding yet one more boolean member:
>
> bool isApplier; /* True => This is an Applier-OI */
> bool isAug; /* True => handle internal to OI augmented CCB */
> bool isBusy; /* True => handle is locked by a thread until a
> function execution is done */
> bool isProto46; /* True => If true then the server side supported
> protocol46 at handle initialize */ <=========
> .....
> } IMMA_CLIENT_NODE;
>
> This means that the OI clients that initialize early will never use
> the new protocol46 messages
> when they initialize before upgrade to 46 is completed. That is until
> they restart or re-initialize
> a new OI handle AFTER the upgrade to 46 is done.
>
> Again this is not a problem.
> After the upgrade is complete and forever thereafter on that system,
> all OI clients will
> get this flag set, even after a cluster restart.
>
> I prefer such a solution because it is cleaner and generic, can be
> used also to support other
> upgrade issues at the client, independently of this issue and avoids
> adding lots of code that is
> never used except during a very short period in the life of the IMM code.
That's what I tried to do in patch #3.
> Both IMM library and IMM service on a node come together with the same
> OpenSAF version. It cannot happen that on one node we have different
> versions between the library and the service. It may happen that there
> are different OpenSAF versions between nodes, but it's handled by
> protocols.
> So, in this case, proto47 on library side is always TRUE, since it's
> always supported on the node.
The proto47 is enabled when the upgrade completed, but during upgrade
proto47 is disabled and the handles initialized at that time will have
proto47 FALSE. That will avoid IMM library sending out new message types
during upgrade.
> Protocol (not only 47) can be enabled and disabled during IMM handle
> lifetime.
> A check if a protocol is allowed during a handle initialization and
> keeping its state in the library, it does not guarantee that the
> protocol is allowed all the time during the handle lifetime.
The main purpose of proto47 on library side is to avoid sending out new
message types during upgrade. When the upgrade completed, all nodes are
now in new version and can handle both old message types and new message
types.
> Another issue is setting version to "_.0.0". Handle initialization
> functions should always return the latest supported version, which is
> in this A.2.16, or return appropriated error code.
The 'version' returned from IMM service is only used to determine
proto47 (TRUE or FALSE). It will not affect the version of the library
like isImmA2b, isImmA2d...
Best Regards,
Hung Nguyen
DEK Technologies
------------------------------------------------------------------------
*From:* Zoran Milinkovic
*Sent:* Thursday, May 07, 2015 10:06PM
*To:* Hung Nguyen, Reddy Neelakanta Reddy Peddavandla, opensaf-devel
*Subject:* RE: [devel] [PATCH 0 of 5] Review Request for imm: remove all
use of SaNameT from IMM server side and messages [#969]
Hi,
Agree to Neelakanta. IMMA should not depend on internal service protocol.
Only what is relevant for the library is a version.
New message types should be handled by IMM service.
Both IMM library and IMM service on a node come together with the same OpenSAF
version. It cannot happen that on one node we have different versions between
the library and the service. It may happen that there are different OpenSAF
versions between nodes, but it's handled by protocols.
So, in this case, proto47 on library side is always TRUE, since it's always
supported on the node.
Protocol (not only 47) can be enabled and disabled during IMM handle lifetime.
A check if a protocol is allowed during a handle initialization and keeping its
state in the library, it does not guarantee that the protocol is allowed all
the time during the handle lifetime.
Another issue is setting version to "_.0.0". Handle initialization functions
should always return the latest supported version, which is in this A.2.16, or
return appropriated error code.
Best regards,
Zoran
-----Original Message-----
From: Hung Nguyen [mailto:[email protected]]
Sent: Thursday, May 07, 2015 4:46 AM
To: Neelakanta Reddy; [email protected]
Subject: Re: [devel] [PATCH 0 of 5] Review Request for imm: remove all use of
SaNameT from IMM server side and messages [#969]
Hi Neel,
The new member isProto47 of IMMA_CLIENT_NODE is not the same with the members
like isImmA2b, isImmA2d...
It is used to check if the proto47 is enabled at handle initialization.
Since only IMMND knows about proto47 so clients have to get that from IMMND.
Best Regards,
Hung Nguyen
DEK Technologies
------------------------------------------------------------------------
*From:* Reddy Neelakanta Reddy Peddavandla
*Sent:* Monday, May 04, 2015 6:51PM
*To:* opensaf-devel
*Subject:* Re: [devel] [PATCH 0 of 5] Review Request for imm: remove all use of
SaNameT from IMM server side and messages [#969]
Hi Hung,
Reviewed the patches,
Apart from AndersBj comments, following are the comments for patch 3:
Why is the API version read from IMM server.
API versions should be set similarly how it is done for previous IMMA
versions.
/Neel.
On Thursday 16 April 2015 12:50 PM, Hung Nguyen wrote:
> Summary: imm: remove all use of SaNameT from IMM server side and
> messages [#969] Review request for Trac Ticket(s): 969 Peer
> Reviewer(s): AndersBj, Neelakanta, Zoran Pull request to:
> Affected branch(es): default(4.7)
> Development branch: default(4.7)
>
> --------------------------------
> Impacted area Impact y/n
> --------------------------------
> Docs n
> Build system n
> RPM/packaging n
> Configuration files n
> Startup scripts n
> SAF services n
> OpenSAF services y
> Core libraries n
> Samples n
> Tests n
> Other n
>
>
> Comments (indicate scope for each "y" above):
> ---------------------------------------------
>
> changeset f71cf86bfefaea3ab1d2cc15895a9fca4e906f31
> Author: Hung Nguyen <[email protected]>
> Date: Mon, 13 Apr 2015 13:23:45 +0700
>
> imm: Remove SaNameT from IMMND event handling routines [#969]
>
> Replace SaNameT local variables with char*.
>
> changeset 26a8cbdd75fb4d8bcdc52cc2da25c6d3ae9250ed
> Author: Hung Nguyen <[email protected]>
> Date: Mon, 13 Apr 2015 14:06:44 +0700
>
> imm: Add OPENSAF_IMM_FLAG_PRT47_ALLOW value for noStdFlags [#969]
>
> Add OPENSAF_IMM_FLAG_PRT47_ALLOW value for noStdFlags.
>
> changeset 70b2f04d94cb82b6ef38149043243d980400a94c
> Author: Hung Nguyen <[email protected]>
> Date: Mon, 13 Apr 2015 18:48:16 +0700
>
> imm: Check if server side supports protocol47 at handle initialization
> [#969]
>
> Add new member 'version' to IMMSV_ND2A_INIT_RSP. When protocol47 is
> supported, it is set to 'A.2.16'. Otherwise, it is set to '_.0.0'.
>
> Add new member 'isProto47' to IMMA_CLIENT_NODE. If protocol47 is
> supported
> at handle initialization, 'isProto47' is set to true.
>
> The clients initialized before protocol47 will never use the new
> protocol47
> messages.
>
> changeset 9305a943af095551557b918545019022c85a9d21
> Author: Hung Nguyen <[email protected]>
> Date: Tue, 14 Apr 2015 17:11:44 +0700
>
> imm: Replace use of SaNameT from IMMSV_OM_ADMIN_OWNER_INITIALIZE
> [#969]
>
> Define new message types: IMMD_EVT_ND2D_ADMINIT_REQ_2 and
> IMMND_EVT_D2ND_ADMINIT_2. IMMND_EVT_A2ND_IMM_ADMINIT messages are only
> sent
> in scope of a node so no need to define a new message type.
>
> Add new member 'stringAdminOwnerName' to
> IMMSV_OM_ADMIN_OWNER_INITIALIZE.
> 'stringAdminOwnerName' is used in IMMND_EVT_A2ND_IMM_ADMINIT and new
> message
> types.
>
> IMMND will use IMMD_EVT_ND2D_ADMINIT_REQ_2 messages if protocol47 is
> enabled
> and IMMD_EVT_ND2D_ADMINIT_REQ messages if protocol47 is disabled.
>
> When receiving an IMMD_EVT_ND2D_ADMINIT_REQ message, IMMD will
> broadcast an
> IMMND_EVT_D2ND_ADMINIT message. When receiving an
> IMMD_EVT_ND2D_ADMINIT_REQ_2 message, IMMD will broadcast an
> IMMND_EVT_D2ND_ADMINIT_2 message.
>
> ImmModel now has to handle both old and new messages. The message type
> is
> detected by checking if 'name' member of
> IMMSV_OM_ADMIN_OWNER_INITIALIZE is
> empty or not.
>
> changeset 82ed5258c4ca9a1810a1915cf2af7771a5578759
> Author: Hung Nguyen <[email protected]>
> Date: Wed, 15 Apr 2015 11:17:32 +0700
>
> imm: Replace use of SaNameT from IMMSV_OI_CCB_UPCALL_RSP [#969]
>
> Define new message types: IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_3,
> IMMND_EVT_A2ND_CCB_OBJ_DELETE_RSP_4 and
> IMMND_EVT_A2ND_OI_CCB_AUG_INIT_2.
>
> Add new member 'stringName' to IMMSV_OI_CCB_UPCALL_RSP. 'stringName' is
> used
> in new message types.
>
> Clients will use new messages when 'isProto47' is true. Otherwise, old
> messages are used.
>
> ImmModel now has to handle both old and new messages. The message type
> is
> detected by checking if 'name' member of IMMSV_OI_CCB_UPCALL_RSP is
> empty or
> not.
>
>
> Complete diffstat:
> ------------------
> osaf/libs/agents/saf/imma/imma_cb.h | 1 +
> osaf/libs/agents/saf/imma/imma_oi_api.c | 16 ++++-
> osaf/libs/agents/saf/imma/imma_om_api.c | 14 +++-
> osaf/libs/agents/saf/imma/imma_proc.c | 16 ++++-
> osaf/libs/common/immsv/immsv_evt.c | 336
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------
> osaf/libs/common/immsv/include/immsv_api.h | 1 +
> osaf/libs/common/immsv/include/immsv_evt.h | 8 ++
> osaf/libs/common/immsv/include/immsv_evt_model.h | 2 +
> osaf/services/saf/immsv/immd/immd_evt.c | 19 +++++-
> osaf/services/saf/immsv/immnd/ImmModel.cc | 85
> ++++++++++++++++++++++++----
> osaf/services/saf/immsv/immnd/ImmModel.hh | 3 +-
> osaf/services/saf/immsv/immnd/immnd_evt.c | 77
> +++++++++++++++++++++-----
> osaf/services/saf/immsv/immnd/immnd_init.h | 7 +-
> 13 files changed, 418 insertions(+), 167 deletions(-)
>
>
> Testing Commands:
> -----------------
>
>
> Testing, Expected Results:
> --------------------------
>
>
> Conditions of Submission:
> -------------------------
> Ack from reviewers
>
>
> Arch Built Started Linux distro
> -------------------------------------------
> mips n n
> mips64 n n
> x86 n n
> x86_64 n n
> powerpc n n
> powerpc64 n n
>
>
> Reviewer Checklist:
> -------------------
> [Submitters: make sure that your review doesn't trigger any
> checkmarks!]
>
>
> Your checkin has not passed review because (see checked entries):
>
> ___ Your RR template is generally incomplete; it has too many blank entries
> that need proper data filled in.
>
> ___ You have failed to nominate the proper persons for review and push.
>
> ___ Your patches do not have proper short+long header
>
> ___ You have grammar/spelling in your header that is unacceptable.
>
> ___ You have exceeded a sensible line length in your headers/comments/text.
>
> ___ You have failed to put in a proper Trac Ticket # into your commits.
>
> ___ You have incorrectly put/left internal data in your comments/files
> (i.e. internal bug tracking tool IDs, product names etc)
>
> ___ You have not given any evidence of testing beyond basic build tests.
> Demonstrate some level of runtime or other sanity testing.
>
> ___ You have ^M present in some of your files. These have to be removed.
>
> ___ You have needlessly changed whitespace or added whitespace crimes
> like trailing spaces, or spaces before tabs.
>
> ___ You have mixed real technical changes with whitespace and other
> cosmetic code cleanup changes. These have to be separate commits.
>
> ___ You need to refactor your submission into logical chunks; there is
> too much content into a single commit.
>
> ___ You have extraneous garbage in your review (merge commits etc)
>
> ___ You have giant attachments which should never have been sent;
> Instead you should place your content in a public tree to be pulled.
>
> ___ You have too many commits attached to an e-mail; resend as threaded
> commits, or place in a public tree for a pull.
>
> ___ You have resent this content multiple times without a clear indication
> of what has changed between each re-send.
>
> ___ You have failed to adequately and individually address all of the
> comments and change requests that were proposed in the initial review.
>
> ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)
>
> ___ Your computer have a badly configured date and time; confusing the
> the threaded patch review.
>
> ___ Your changes affect IPC mechanism, and you don't present any results
> for in-service upgradability test.
>
> ___ Your changes affect user manual and documentation, your patch series
> do not contain the patch that updates the Doxygen manual.
>
>
> ----------------------------------------------------------------------
> -------- BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
> Develop your own process in accordance with the BPMN 2 standard Learn
> Process modeling best practices with Bonita BPM through live exercises
> http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual-
> event?utm_
> source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
> _______________________________________________
> Opensaf-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud Widest
out-of-the-box monitoring support with 50+ applications Performance metrics,
stats and reports that give you Actionable Insights Deep dive visibility with
transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud Widest
out-of-the-box monitoring support with 50+ applications Performance metrics,
stats and reports that give you Actionable Insights Deep dive visibility with
transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
