Looks like some other code ended up in this patch?

On 04/06/2017 05:30 PM, Alex Jones wrote:
> Summary: smfd: fix race condition during campaign commit
> Review request for Trac Ticket(s): 2413
> Peer Reviewer(s): lennart, rafael, neel
> Pull request to:
> Affected branch(es): default
> Development branch:
>
> --------------------------------
> 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):
> ---------------------------------------------
>
> changeset b51342e4460065d8a4416442e74f20d3db9c0a13
> Author:       Alex Jones <ajo...@genband.com>
> Date: Thu, 06 Apr 2017 11:22:19 -0400
>
>       smfd: fix race condition when detecting async failures [#2413]
>
>       smfd core dumps during commit of campaign.
>
>       If an AMF SU under maintenance fails right as the campaign commit is 
> done,
>       there is a race condition present. Before SMF clears the
>       suMaintenaceCampaign attribute of the SU, if the SU fails, it will send 
> a
>       notification. Meanwhile, the commit deletes upgrade campaign pointer 
> inside
>       smfd. If the deletion of the campaign pointer inside smfd occurs before 
> it
>       receives the NTF event a crash will occur because the campaign pointer 
> is
>       gone.
>
>       Solution is to always process NTF events before processing the 
> termination
>       of the campaign. The campaign termination code sets "m_running" to 
> false,
>       and deletes the pointer. This should always be last in the poll loop so 
> that
>       the loop will exit immediately without processing any NTF events (or 
> other
>       future events.)
>
>
> Complete diffstat:
> ------------------
>   src/lck/Makefile.am               |    8 +-
>   src/lck/agent/gla_api.c           |   96 +++++++++++++
>   src/lck/agent/gla_cb.h            |    4 +
>   src/lck/agent/gla_evt.h           |   11 +-
>   src/lck/agent/gla_init.c          |   26 +++
>   src/lck/agent/gla_mds.c           |   55 +++++++
>   src/lck/common/glsv_defs.h        |    9 +-
>   src/lck/lckd/gld_api.c            |   27 +++-
>   src/lck/lckd/gld_cb.h             |    2 +-
>   src/lck/lckd/gld_evt.c            |  105 +++++++++-----
>   src/lck/lckd/gld_evt.h            |   16 ++
>   src/lck/lckd/gld_rsc.c            |    8 +
>   src/lck/lcknd/glnd_agent.c        |   22 +++
>   src/lck/lcknd/glnd_api.c          |   33 +++-
>   src/lck/lcknd/glnd_cb.c           |   10 +
>   src/lck/lcknd/glnd_cb.h           |   12 +
>   src/lck/lcknd/glnd_ckpt.c         |   32 ++++
>   src/lck/lcknd/glnd_client.c       |  175 +++++++++++++++++++++--
>   src/lck/lcknd/glnd_client.h       |   10 +
>   src/lck/lcknd/glnd_evt.c          |  268 
> ++++++++++++++++++++++++++++++-------
>   src/lck/lcknd/glnd_mds.c          |   39 +++++
>   src/lck/lcknd/glnd_mds.h          |    8 +
>   src/lck/lcknd/glnd_res.c          |   54 ++++++-
>   src/lck/lcknd/glnd_res_req.c      |   48 ++++++
>   src/lck/lcknd/glnd_restart.c      |   60 ++++++++-
>   src/smf/smfd/SmfCampaignThread.cc |   16 +-
>   tools/cluster_sim_uml/opensaf     |    4 +-
>   27 files changed, 1014 insertions(+), 144 deletions(-)
>
>
> Testing Commands:
> -----------------
> (1) run a campaign and let it get to completed state
> (2) commit and at the same time kill a component with suMaintenanceCampaign 
> set
>
> Testing, Expected Results:
> --------------------------
> smfd should not core
>
>
> Conditions of Submission:
> -------------------------
>
>
> 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.
>


------------------------------------------------------------------------------
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

Reply via email to