Summary: imm: sPbeRtMutations is updated even when validation for duplicate 
values fails [#2422]
Review request for Ticket(s): 2422
Peer Reviewer(s): Ravi, Anders, Hans, Lennart, Vu
Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE ***
Affected branch(es): develop
Development branch: ticket-2422
Base revision: 3d8e8a959e298ff438fe5a6e7cc02e2404305542
Personal repository: git://git.code.sf.net/u/zvoxdan/review

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

NOTE: Patch(es) contain lines longer than 80 characers

Comments (indicate scope for each "y" above):
---------------------------------------------
*** EXPLAIN/COMMENT THE PATCH SERIES HERE ***

revision fff1ace1d400ec5b56c391d0389f228032f00e7e
Author: Danh Vo <[email protected]>
Date:   Thu, 15 Mar 2018 17:16:56 +0700

imm: sPbeRtMutations is updated even when validation for duplicate values fails 
[#2422]

When immnd receives event to update persistent runtime attribute
from immd, there is 2 loops: the first loop validates the change,
the second one updates the afim object. The root cause comes from
the check (attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) which is
inside the second loop instead of the first loop. So the
validation is still passed even when the attribute does not allow
duplicate value. As a result, sPbeRtMutations is updated in the
second loop.



Complete diffstat:
------------------
 src/imm/immnd/ImmModel.cc | 65 ++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 29 deletions(-)


Testing Commands:
-----------------
Create Test class as below:
  <class name="Test">
        <category>SA_CONFIG</category>
        <rdn>
                <name>test</name>
                <type>SA_STRING_T</type>
                <category>SA_CONFIG</category>
                <flag>SA_INITIALIZED</flag>
        </rdn>
        <attr>
                <name>list</name>
                <type>SA_UINT32_T</type>
                <category>SA_RUNTIME</category>
                <flag>SA_CACHED</flag>
                <flag>SA_PERSISTENT</flag>
                <flag>SA_MULTI_VALUE</flag>
                <flag>SA_NO_DUPLICATES</flag>
        </attr>
  </class>
Create object test=1 by command:
  immcfg -c Test test=1
Create an implementer which has:
  - saImmOiRtAttrUpdateCallback returns SA_AIS_OK.
  - saImmOiAdminOperationCallback does saImmOiRtObjectUpdate_2() command to 
    update "list" attribute as bellow sequence:
       + Add value=9 to 'list' attribute
       + Add value=9 to 'list' attribute
       + Add value=10 to 'list' attribute
Perform admin operation on test=1 by command:
  immadm -o 1 test=1
See the result if it is expected.


Testing, Expected Results:
--------------------------
2018-03-15 15:43:09.889 SC-1 2422-implemter: >> AdminOperationCallbackT_2
2018-03-15 15:43:09.890 SC-1 2422-implemter: Add 9 into "list" attribute
2018-03-15 15:43:10.001 SC-1 2422-implemter: saImmOiRtObjectUpdate_2 return 
code: 1
2018-03-15 15:43:10.001 SC-1 2422-implemter: Add 9 into "list" attribute
2018-03-15 15:43:10.004 SC-1 osafimmnd[207]: NO ERR_INVALID_PARAM: multivalued 
attr 'list' with NO_DUPLICATES yet duplicate values provided in rta-update 
call. Object:'test=1'.
2018-03-15 15:43:10.004 SC-1 osafimmnd[207]: WA Got error on non local rt 
object update err: 7
2018-03-15 15:43:10.004 SC-1 2422-implemter: saImmOiRtObjectUpdate_2 return 
code: 7
2018-03-15 15:43:10.004 SC-1 2422-implemter: Add 10 into "list" attribute
2018-03-15 15:43:10.050 SC-1 2422-implemter: saImmOiRtObjectUpdate_2 return 
code: 1
2018-03-15 15:43:10.051 SC-1 2422-implemter: << AdminOperationCallbackT_2


Conditions of Submission:
-------------------------
Ack from one of 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 ~/.gitconfig file (i.e. user.name, user.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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to