ack for the series.
Perhaps the trace message could be phrased a bit better to indicate clc cmd of
the same type is in progress.
TRACE("Another clc cmd has been in progress”);
> On 30 Oct 2015, at 9:11 PM, Nagendra Kumar <[email protected]> wrote:
>
> Ack for the series.
>
> Thanks
> -Nagu
>> -----Original Message-----
>> From: Minh Hon Chau [mailto:[email protected]]
>> Sent: 28 October 2015 06:47
>> To: [email protected]; Nagendra Kumar; Praveen Malviya;
>> [email protected]
>> Cc: [email protected]
>> Subject: [PATCH 0 of 2] Review Request for Do not run same clc commands
>> at once [#1557]
>>
>> Summary: amfnd: Do not run same clc commands at once [#1557]
>> Review request for Trac Ticket(s): 1557
>> Peer Reviewer(s): AMF devs
>> Pull request to: AMF maintainers
>> Affected branch(es): all
>> Development branch: default
>>
>> --------------------------------
>> Impacted area Impact y/n
>> --------------------------------
>> Docs n
>> Build system n
>> RPM/packaging n
>> Configuration files n
>> Startup scripts n
>> SAF services y
>> OpenSAF services n
>> Core libraries n
>> Samples n
>> Tests n
>> Other n
>>
>>
>> Comments (indicate scope for each "y" above):
>> ---------------------------------------------
>> Review series contains 2 patches:
>> - one is to fix the bug reported in #1557
>> - pickup little things for code clean up (for default branch only)
>>
>>
>> changeset ea670980b3604a3d84a7fcd19f7b091511165894
>> Author: Minh Hon Chau <[email protected]>
>> Date: Wed, 28 Oct 2015 11:52:21 +1100
>>
>> amfnd: Do not run same clc commands at once [#1557]
>>
>> Currently if two errors happen at once, amfnd calls two clc cleanup
>> commands
>> in a row, and component fails into INSTANTIATION-FAILED state
>>
>> As two clc cleanup commands for same component are called, that
>> means two
>> flows of component life cycle are running at the same time. After the
>> first
>> cleanup finishes, amfnd starts clc initiate command. For the reason
>> mostly
>> due to timing, while amfnd is waiting for the return of clc initiate,
>> amfnd
>> receives the return of second clc cleanup. Upon receiving the return
>> of
>> second clc cleanup, the next step is that amfnd will start another clc
>> initiate if the max retry of clc initiate is large enough. But if the
>> max
>> retry is 2, component fails into INSTANTIATION-FALED state, which
>> should not
>> happen.
>>
>> The cause of this issue is mainly because amfnd lets two clc
>> commands
>> running concurrently. The second clc command is redundant while
>> the first
>> one is in progress. Any errors happen during clc command execution,
>> it
>> should be detected in the first one. The patch avoids the same clc
>> commands
>> running in a row if amfnd has not got clc_resp (which indicates
>> timeout/error/ok) from the others.
>>
>> changeset b8881ec872c31d3ed9f776ec012739fa9bd0147f
>> Author: Minh Hon Chau <[email protected]>
>> Date: Wed, 28 Oct 2015 11:55:59 +1100
>>
>> amfnd: Cleaning code as part of #1557
>>
>> The patch for cleaning code after fix of #1557: Ensure
>> TRACE_LEAVE2 is
>> called Remove unused env_attr_val
>>
>>
>> Complete diffstat:
>> ------------------
>> osaf/services/saf/amf/amfnd/clc.cc | 24
>> ++++++++++++++----------
>> osaf/services/saf/amf/amfnd/include/avnd_comp.h | 3 ++-
>> 2 files changed, 16 insertions(+), 11 deletions(-)
>>
>>
>> Testing Commands:
>> -----------------
>> Perform the steps reported in ticket
>>
>>
>> Testing, Expected Results:
>> --------------------------
>> Component does not fail into INSTANTIATION-FALED
>>
>>
>> Conditions of Submission:
>> -------------------------
>> ack from at least 3 reviewers
>>
>>
>> Arch Built Started Linux distro
>> -------------------------------------------
>> mips n n
>> mips64 n n
>> x86 n n
>> x86_64 y y
>> 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.
>>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
