Hi zoran,
Ok, I overlooked the functionality part immnd_evt_ccb_augment_init.
Thanks,
Neel.
On Friday 18 September 2015 06:47 PM, Zoran Milinkovic wrote:
> Hi Neelakanta,
>
> 1. free() is safe for NULL value and there is no need for extra check.
>
> 2. It is not missed.
> OM for ccbAugmentInit is OI callback.
> Error string is attached to the main CCB (not to augmented CCB), and sending
> error string over MDS is skipped:
> if(ccb->mVeto != SA_AIS_OK) {
> TRACE("Ccb %u is already in an error state %u, can not accept
> augmentation",
> ccbId, ccb->mVeto);
> err = SA_AIS_ERR_FAILED_OPERATION; /*ccb->mVeto;*/
> setCcbErrorString(ccb, IMM_RESOURCE_ABORT "CCB is in an error
> state");
> goto done;
> }
> This error string is sent to OM (main CCB) when OI callback is done.
>
> Best regards,
> Zoran
>
> -----Original Message-----
> From: Neelakanta Reddy [mailto:[email protected]]
> Sent: Friday, September 18, 2015 1:31 PM
> To: Zoran Milinkovic
> Cc: [email protected]
> Subject: Re: [PATCH 0 of 1] Review Request for imm: classify abort error
> strings and prefix existing error strings [#744]
>
> Hi zoran,
>
> Reviewed and tested the patch.
> Ack with the following comments.
>
> comments:
>
> 1. For immnd_evt_proc_ccb_obj_modif and immnd_evt_proc_ccb_obj_creat
>
> good to have if(errStrList.name.buf) before freeing
>
> +
> + free(errStrList.name.buf);
>
> 2. The following function is missed in the new patch the error strings are
> not sent to IMMA OM in the reply from IMMND for immnd_evt_ccb_augment_init
>
> /Neel.
>
> On Wednesday 16 September 2015 07:58 PM, Zoran Milinkovic wrote:
>> Summary: imm: classify abort error strings and prefix existing error
>> strings [#744] Review request for Trac Ticket(s): 744 Peer
>> Reviewer(s): Anders, Neelakanta, Hung Pull request to: Zoran 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 y
>> OpenSAF services n
>> Core libraries n
>> Samples n
>> Tests n
>> Other n
>>
>>
>> Comments (indicate scope for each "y" above):
>> ---------------------------------------------
>>
>> changeset b4e9607716953b582020ed9166298d7b36d403a8
>> Author: Zoran Milinkovic <[email protected]>
>> Date: Wed, 16 Sep 2015 16:21:17 +0200
>>
>> imm: classify abort error strings and prefix existing error strings
>> [#744]
>>
>> The patch set prefix "IMM:" to all error string that come from IMM.
>> Based on
>> CCB abort type (resource or validation abort), error strings are
>> prefixed
>> with "IMM: Resource abort:" or "IMM: Validation abort:"
>>
>>
>> Complete diffstat:
>> ------------------
>> osaf/services/saf/immsv/immnd/ImmModel.cc | 129
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------
>> osaf/services/saf/immsv/immnd/ImmModel.hh | 5 +++++
>> osaf/services/saf/immsv/immnd/immnd_evt.c | 125
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
>> osaf/services/saf/immsv/immnd/immnd_init.h | 6 ++++++
>> 4 files changed, 232 insertions(+), 33 deletions(-)
>>
>>
>> Testing Commands:
>> -----------------
>>
>>
>> Testing, Expected Results:
>> --------------------------
>> Test IMM that IMM returns correct error strings. Testing should be mostly
>> focused on testing error strings when CCB is aborted.
>>
>>
>> Conditions of Submission:
>> -------------------------
>> Ack from Neelakanta, Hung and Anders
>>
>>
>> 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.
>>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
