Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Vu Minh Nguyen]

Please see my comments inline.

Regards, Vu.

>-Original Message-
>From: Mathivanan Naickan Palanivelu [mailto:mathi.naic...@oracle.com]
>Sent: Tuesday, March 15, 2016 1:29 PM
>To: Vu Minh Nguyen; Anders Widell; Lennart Lund
>Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
Garcia
>Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
about
>origin of log record [#1480]
>
>Please see a comment inline:
>
>> -Original Message-
>> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
>> Sent: Tuesday, March 15, 2016 11:53 AM
>> To: Mathivanan Naickan Palanivelu; Anders Widell; Lennart Lund
>> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
>> Garcia
>> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
about
>> origin of log record [#1480]
>>
>> Hi Mathi,
>>
>> See my responses inline, with [Vu].
>>
>> Regards, Vu.
>>
>> >-Original Message-
>> >From: Mathivanan Naickan Palanivelu [mailto:mathi.naic...@oracle.com]
>> >Sent: Tuesday, March 15, 2016 1:07 PM
>> >To: Anders Widell; Vu Minh Nguyen; Lennart Lund
>> >Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
>> Garcia
>> >Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
>> about
>> >origin of log record [#1480]
>> >
>> >Hi,
>> >
>> >Comments inline:
>> >
>> >> -Original Message-
>> >> From: Anders Widell [mailto:anders.wid...@ericsson.com]
>> >> Sent: Monday, March 14, 2016 4:52 PM
>> >> To: Vu Minh Nguyen; Lennart Lund; Mathivanan Naickan Palanivelu
>> >> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge
>> >> Pacheco Garcia
>> >> Subject: Re: [PATCH 0 of 1] Review Request for log: Extend
>> >> information about origin of log record [#1480]
>> >>
>> >> See my comments inline.
>> >>
>> >> regards,
>> >> Anders Widell
>> >>
>> >> On 03/08/2016 03:49 AM, Vu Minh Nguyen wrote:
>> >> > Hi Lennart,
>> >> >
>> >> > Please see my responses inline, with [Vu].
>> >> >
>> >> > Regards, Vu.
>> >> >
>> >> >> -Original Message-
>> >> >> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
>> >> >> Sent: Thursday, March 03, 2016 10:38 PM
>> >> >> To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
>> >> >> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge
>> >> >> Pacheco
>> >> > Garcia
>> >> >> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend
>> >> >> information
>> >> > about
>> >> >> origin of log record [#1480]
>> >> >>
>> >> >> Hi Vu
>> >> >>
>> >> >> My comments:
>> >> >>
>> >> >> ---
>> >> >> logtest.c
>> >> >> get_attr_value()
>> >> >> This function looks like it can get a value from any attribute in
>> >> >> any
>> >> > object but
>> >> >> this does not seems to be the case.
>> >> >> It can only get values from some specific objects and also not for
>> >> >> all
>> >> > attributes
>> >> >> in those objects. This is very unclean code and should be improved.
>> >> >> Also
>> >> > the
>> >> >> test code should be kept as clean as possible especially global
>> functions.
>> >> >> There are several ways of handling this e.g.
>> >> >> A function that takes the object name and the attribute name (as a
>> >> >> C
>> >> > strings)
>> >> >> as in parameters and has a void pointer for the fetched value.
>> >> >> Since no attributes in the log service can be multivalue it should
>> >> >> be ok to always
>> >> > give
>> >> >> value[0] as output. This is also a void pointer. The calling
>> >> >> function
>> >> > should know
>> >> >> how to typecast the fetched value to the correct type. The only
>> >> >> limitation
>> >> > for
>> >> >> this function is that it will only return the first value in case
>> >> >> of
>> >> > multivalue but
>> >> >> this should be no problem.
>> >> >>
>> >> > [Vu] Thanks. I propose to improve that in an enhancement ticket.
>> >> > With that ticket, we can add points to be improved.
>> >> >
>> >> >> ---
>> >> >> gcfg_classes.xml
>> >> >> gcfg_objects.xml
>> >> >>
>> >> >> Has to be moved to an appropriate directory. Cannot be placed with
>> >> >> the log service. Talk to Anders W.
>> >> >> Maybe other reviewers also have comments about this.
>> >> >>
>> >> > [Vu] Yes, I think so. Anders & Mathi may give comments on this.
>> >> [AndersW] We have one existing service that could be a candidate for
>> >> owning these classes: NID. So one idea is to put them in the
>> >> services/infrastructure/nid/config/ directory. Another alternative is
>> >> to
>> create
>> >> a new config directory at the global scope: either osaf/config/ or
>> >> osaf/services/config/. I don't have any strong preference for any of
>> these
>> >> alternatives, but if I must pick one I think it would be
>> osaf/services/config/.
>> >[Mathi]
>> >Okay.
>> [Vu] I will create the new directory "config" under osaf/services
>>
>> >
>> >And also to rename the files to osaf_globalconfig_classes.xml and
>> 

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Mathivanan Naickan Palanivelu]

Please see a comment inline:

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: Tuesday, March 15, 2016 11:53 AM
> To: Mathivanan Naickan Palanivelu; Anders Widell; Lennart Lund
> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> Garcia
> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information about
> origin of log record [#1480]
> 
> Hi Mathi,
> 
> See my responses inline, with [Vu].
> 
> Regards, Vu.
> 
> >-Original Message-
> >From: Mathivanan Naickan Palanivelu [mailto:mathi.naic...@oracle.com]
> >Sent: Tuesday, March 15, 2016 1:07 PM
> >To: Anders Widell; Vu Minh Nguyen; Lennart Lund
> >Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> Garcia
> >Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
> about
> >origin of log record [#1480]
> >
> >Hi,
> >
> >Comments inline:
> >
> >> -Original Message-
> >> From: Anders Widell [mailto:anders.wid...@ericsson.com]
> >> Sent: Monday, March 14, 2016 4:52 PM
> >> To: Vu Minh Nguyen; Lennart Lund; Mathivanan Naickan Palanivelu
> >> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge
> >> Pacheco Garcia
> >> Subject: Re: [PATCH 0 of 1] Review Request for log: Extend
> >> information about origin of log record [#1480]
> >>
> >> See my comments inline.
> >>
> >> regards,
> >> Anders Widell
> >>
> >> On 03/08/2016 03:49 AM, Vu Minh Nguyen wrote:
> >> > Hi Lennart,
> >> >
> >> > Please see my responses inline, with [Vu].
> >> >
> >> > Regards, Vu.
> >> >
> >> >> -Original Message-
> >> >> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> >> >> Sent: Thursday, March 03, 2016 10:38 PM
> >> >> To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
> >> >> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge
> >> >> Pacheco
> >> > Garcia
> >> >> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend
> >> >> information
> >> > about
> >> >> origin of log record [#1480]
> >> >>
> >> >> Hi Vu
> >> >>
> >> >> My comments:
> >> >>
> >> >> ---
> >> >> logtest.c
> >> >> get_attr_value()
> >> >> This function looks like it can get a value from any attribute in
> >> >> any
> >> > object but
> >> >> this does not seems to be the case.
> >> >> It can only get values from some specific objects and also not for
> >> >> all
> >> > attributes
> >> >> in those objects. This is very unclean code and should be improved.
> >> >> Also
> >> > the
> >> >> test code should be kept as clean as possible especially global
> functions.
> >> >> There are several ways of handling this e.g.
> >> >> A function that takes the object name and the attribute name (as a
> >> >> C
> >> > strings)
> >> >> as in parameters and has a void pointer for the fetched value.
> >> >> Since no attributes in the log service can be multivalue it should
> >> >> be ok to always
> >> > give
> >> >> value[0] as output. This is also a void pointer. The calling
> >> >> function
> >> > should know
> >> >> how to typecast the fetched value to the correct type. The only
> >> >> limitation
> >> > for
> >> >> this function is that it will only return the first value in case
> >> >> of
> >> > multivalue but
> >> >> this should be no problem.
> >> >>
> >> > [Vu] Thanks. I propose to improve that in an enhancement ticket.
> >> > With that ticket, we can add points to be improved.
> >> >
> >> >> ---
> >> >> gcfg_classes.xml
> >> >> gcfg_objects.xml
> >> >>
> >> >> Has to be moved to an appropriate directory. Cannot be placed with
> >> >> the log service. Talk to Anders W.
> >> >> Maybe other reviewers also have comments about this.
> >> >>
> >> > [Vu] Yes, I think so. Anders & Mathi may give comments on this.
> >> [AndersW] We have one existing service that could be a candidate for
> >> owning these classes: NID. So one idea is to put them in the
> >> services/infrastructure/nid/config/ directory. Another alternative is
> >> to
> create
> >> a new config directory at the global scope: either osaf/config/ or
> >> osaf/services/config/. I don't have any strong preference for any of
> these
> >> alternatives, but if I must pick one I think it would be
> osaf/services/config/.
> >[Mathi]
> >Okay.
> [Vu] I will create the new directory "config" under osaf/services
> 
> >
> >And also to rename the files to osaf_globalconfig_classes.xml and
> >osaf_globalconfig_objects.xml
> [Vu] Ok. I will rename them.
> 
> >
> >B.T.W A general comment, Please do breakdown patches into smaller
> >logical changesets whenever possible.
> [Vu] Ok. Will do it from now on for huge patch file.
> I am going to resent the patch out for review with following patches:
> 1) One patch contains xml files and new added directory
> 2) One patch contain changes in log services and updated readme file
> 3) One patch contains test code

If you indeed have them as separate changesets you may perhaps just push them 

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Vu Minh Nguyen]

Hi Mathi,

See my responses inline, with [Vu].

Regards, Vu.

>-Original Message-
>From: Mathivanan Naickan Palanivelu [mailto:mathi.naic...@oracle.com]
>Sent: Tuesday, March 15, 2016 1:07 PM
>To: Anders Widell; Vu Minh Nguyen; Lennart Lund
>Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
Garcia
>Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
about
>origin of log record [#1480]
>
>Hi,
>
>Comments inline:
>
>> -Original Message-
>> From: Anders Widell [mailto:anders.wid...@ericsson.com]
>> Sent: Monday, March 14, 2016 4:52 PM
>> To: Vu Minh Nguyen; Lennart Lund; Mathivanan Naickan Palanivelu
>> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
>> Garcia
>> Subject: Re: [PATCH 0 of 1] Review Request for log: Extend information
>> about origin of log record [#1480]
>>
>> See my comments inline.
>>
>> regards,
>> Anders Widell
>>
>> On 03/08/2016 03:49 AM, Vu Minh Nguyen wrote:
>> > Hi Lennart,
>> >
>> > Please see my responses inline, with [Vu].
>> >
>> > Regards, Vu.
>> >
>> >> -Original Message-
>> >> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
>> >> Sent: Thursday, March 03, 2016 10:38 PM
>> >> To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
>> >> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge
>> >> Pacheco
>> > Garcia
>> >> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend
>> >> information
>> > about
>> >> origin of log record [#1480]
>> >>
>> >> Hi Vu
>> >>
>> >> My comments:
>> >>
>> >> ---
>> >> logtest.c
>> >> get_attr_value()
>> >> This function looks like it can get a value from any attribute in any
>> > object but
>> >> this does not seems to be the case.
>> >> It can only get values from some specific objects and also not for
>> >> all
>> > attributes
>> >> in those objects. This is very unclean code and should be improved.
>> >> Also
>> > the
>> >> test code should be kept as clean as possible especially global
functions.
>> >> There are several ways of handling this e.g.
>> >> A function that takes the object name and the attribute name (as a C
>> > strings)
>> >> as in parameters and has a void pointer for the fetched value. Since
>> >> no attributes in the log service can be multivalue it should be ok to
>> >> always
>> > give
>> >> value[0] as output. This is also a void pointer. The calling function
>> > should know
>> >> how to typecast the fetched value to the correct type. The only
>> >> limitation
>> > for
>> >> this function is that it will only return the first value in case of
>> > multivalue but
>> >> this should be no problem.
>> >>
>> > [Vu] Thanks. I propose to improve that in an enhancement ticket.
>> > With that ticket, we can add points to be improved.
>> >
>> >> ---
>> >> gcfg_classes.xml
>> >> gcfg_objects.xml
>> >>
>> >> Has to be moved to an appropriate directory. Cannot be placed with
>> >> the log service. Talk to Anders W.
>> >> Maybe other reviewers also have comments about this.
>> >>
>> > [Vu] Yes, I think so. Anders & Mathi may give comments on this.
>> [AndersW] We have one existing service that could be a candidate for
>> owning these classes: NID. So one idea is to put them in the
>> services/infrastructure/nid/config/ directory. Another alternative is to
create
>> a new config directory at the global scope: either osaf/config/ or
>> osaf/services/config/. I don't have any strong preference for any of
these
>> alternatives, but if I must pick one I think it would be
osaf/services/config/.
>[Mathi]
>Okay.
[Vu] I will create the new directory "config" under osaf/services

>
>And also to rename the files to osaf_globalconfig_classes.xml and
>osaf_globalconfig_objects.xml
[Vu] Ok. I will rename them.

>
>B.T.W A general comment, Please do breakdown patches into smaller logical
>changesets whenever possible.
[Vu] Ok. Will do it from now on for huge patch file.
I am going to resent the patch out for review with following patches:
1) One patch contains xml files and new added directory
2) One patch contain changes in log services and updated readme file
3) One patch contains test code

>
>Thanks,
>Mathi.
>
>> Mathi, do you have any preference or other suggestion?
>>
>> >
>> >> ---
>> >> lgs_imm_gcfg.cc
>> >> I hope you have thoroughly checked this code. This code is a rather
>> >> quickly created prototype code, is not very well tested and had not
>> >> been reviewed
>> > by
>> >> anybody before you got it. It should also be verified with valgrind
>> >> thread
>> > check
>> >> (has not been done)
>> >> - Spelling error on line 60. Replace "snd" with "and"
>> >> - There is no information telling what this function is doing and why
>> >> send_command() function
>> > [Vu] Thanks. I added the description on top of this function.
>> >>
>> >> ---
>> >> lgs_mbcsv.cc
>> >> Remove TODO on line 

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Mathivanan Naickan Palanivelu]

Hi,

Comments inline:

> -Original Message-
> From: Anders Widell [mailto:anders.wid...@ericsson.com]
> Sent: Monday, March 14, 2016 4:52 PM
> To: Vu Minh Nguyen; Lennart Lund; Mathivanan Naickan Palanivelu
> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> Garcia
> Subject: Re: [PATCH 0 of 1] Review Request for log: Extend information
> about origin of log record [#1480]
> 
> See my comments inline.
> 
> regards,
> Anders Widell
> 
> On 03/08/2016 03:49 AM, Vu Minh Nguyen wrote:
> > Hi Lennart,
> >
> > Please see my responses inline, with [Vu].
> >
> > Regards, Vu.
> >
> >> -Original Message-
> >> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> >> Sent: Thursday, March 03, 2016 10:38 PM
> >> To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
> >> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge
> >> Pacheco
> > Garcia
> >> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend
> >> information
> > about
> >> origin of log record [#1480]
> >>
> >> Hi Vu
> >>
> >> My comments:
> >>
> >> ---
> >> logtest.c
> >> get_attr_value()
> >> This function looks like it can get a value from any attribute in any
> > object but
> >> this does not seems to be the case.
> >> It can only get values from some specific objects and also not for
> >> all
> > attributes
> >> in those objects. This is very unclean code and should be improved.
> >> Also
> > the
> >> test code should be kept as clean as possible especially global functions.
> >> There are several ways of handling this e.g.
> >> A function that takes the object name and the attribute name (as a C
> > strings)
> >> as in parameters and has a void pointer for the fetched value. Since
> >> no attributes in the log service can be multivalue it should be ok to
> >> always
> > give
> >> value[0] as output. This is also a void pointer. The calling function
> > should know
> >> how to typecast the fetched value to the correct type. The only
> >> limitation
> > for
> >> this function is that it will only return the first value in case of
> > multivalue but
> >> this should be no problem.
> >>
> > [Vu] Thanks. I propose to improve that in an enhancement ticket.
> > With that ticket, we can add points to be improved.
> >
> >> ---
> >> gcfg_classes.xml
> >> gcfg_objects.xml
> >>
> >> Has to be moved to an appropriate directory. Cannot be placed with
> >> the log service. Talk to Anders W.
> >> Maybe other reviewers also have comments about this.
> >>
> > [Vu] Yes, I think so. Anders & Mathi may give comments on this.
> [AndersW] We have one existing service that could be a candidate for
> owning these classes: NID. So one idea is to put them in the
> services/infrastructure/nid/config/ directory. Another alternative is to 
> create
> a new config directory at the global scope: either osaf/config/ or
> osaf/services/config/. I don't have any strong preference for any of these
> alternatives, but if I must pick one I think it would be 
> osaf/services/config/.
[Mathi]
Okay.

And also to rename the files to osaf_globalconfig_classes.xml and 
osaf_globalconfig_objects.xml

B.T.W A general comment, Please do breakdown patches into smaller logical 
changesets whenever possible.

Thanks,
Mathi.

> Mathi, do you have any preference or other suggestion?
> 
> >
> >> ---
> >> lgs_imm_gcfg.cc
> >> I hope you have thoroughly checked this code. This code is a rather
> >> quickly created prototype code, is not very well tested and had not
> >> been reviewed
> > by
> >> anybody before you got it. It should also be verified with valgrind
> >> thread
> > check
> >> (has not been done)
> >> - Spelling error on line 60. Replace "snd" with "and"
> >> - There is no information telling what this function is doing and why
> >> send_command() function
> > [Vu] Thanks. I added the description on top of this function.
> >>
> >> ---
> >> lgs_mbcsv.cc
> >> Remove TODO on line 1625
> >> Why adding inparameter nodeId to function dec_write_log_async_msg()?
> >> Not used in function Can also be removed from mds_dec() function?
> >> (line 799)
> > [Vu] I removed them.
> >>
> >> ---
> >> README
> >> I suggest some changes in the text added to README
> >>
> >> Original text:
> >> 3. New tokens are added (#1480)
> >> ---
> >> - @Cp: for showing the network name
> >> - @Cq: for showing node name where the log record comes from.
> >>
> >> a) The network name comes from an configurable attribute
> >> `opensafNetworkName`
> >>which belongs to global configurable class `OpensafConfig`.
> >>The attribute can be accessed via DN
> >> `opensafConfigId=opensafGlobalConfig,safApp=OpenSAF`.
> >>
> >>LOG service is an applier to this object class, so that whenever
> >> there
> > is
> >> change in
> >>network name attribute 

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Lennart Lund]

Hi Vu

Ack

Can push when the location of the .xml files is solved and is included in a 
patch to push and all other reviewers has acked.

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 14 mars 2016 11:03
> To: Lennart Lund; Anders Widell; mathi.naic...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> Garcia
> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information about
> origin of log record [#1480]
> 
> Thanks, Lennart.
> 
> I have updated the code. Also, correct the indentation in logtest.c/.h and
> tet_LogOiOps.c
> 
> ➤ sha1sum lgsv_node_originating_r3.patch
> 85446d68fa3785b3eac1a132d6668a09aead2b7d
> lgsv_node_originating_r3.patch
> 
> For location of gcfg_classes.xml and gcfg_objects.xml files, I am looking for
> Anders W & Mathi's feedback.
> 
> Regards, Vu.
> 
> >-Original Message-
> >From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> >Sent: Friday, March 11, 2016 7:56 PM
> >To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
> >Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> >Garcia; Lennart Lund
> >Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
> about
> >origin of log record [#1480]
> >
> >Hi Vu
> >
> >Ack with comment:
> >
> >I think you should fix the get_attr_value() function before pushing.
> >What is the decision for  placement of gcfg_classes.xml and
> gcfg_objects.xml?
> >Must of course also be fixed
> >
> >Thanks
> >Lennart
> >
> >> -Original Message-
> >> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> >> Sent: den 8 mars 2016 03:49
> >> To: Lennart Lund; Anders Widell; mathi.naic...@oracle.com
> >> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> >> Garcia
> >> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
> about
> >> origin of log record [#1480]
> >>
> >> Hi Lennart,
> >>
> >> Please see my responses inline, with [Vu].
> >>
> >> Regards, Vu.
> >>
> >> >-Original Message-
> >> >From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> >> >Sent: Thursday, March 03, 2016 10:38 PM
> >> >To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
> >> >Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge
> Pacheco
> >> Garcia
> >> >Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
> >> about
> >> >origin of log record [#1480]
> >> >
> >> >Hi Vu
> >> >
> >> >My comments:
> >> >
> >> >---
> >> >logtest.c
> >> >get_attr_value()
> >> >This function looks like it can get a value from any attribute in any
> >> object but
> >> >this does not seems to be the case.
> >> >It can only get values from some specific objects and also not for all
> >> attributes
> >> >in those objects. This is very unclean code and should be improved. Also
> >> the
> >> >test code should be kept as clean as possible especially global functions.
> >> >There are several ways of handling this e.g.
> >> >A function that takes the object name and the attribute name (as a C
> >> strings)
> >> >as in parameters and has a void pointer for the fetched value. Since no
> >> >attributes in the log service can be multivalue it should be ok to always
> >> give
> >> >value[0] as output. This is also a void pointer. The calling function
> >> should know
> >> >how to typecast the fetched value to the correct type. The only
> limitation
> >> for
> >> >this function is that it will only return the first value in case of
> >> multivalue but
> >> >this should be no problem.
> >> >
> >> [Vu] Thanks. I propose to improve that in an enhancement ticket.
> >> With that ticket, we can add points to be improved.
> >>
> >> >
> >> >---
> >> >gcfg_classes.xml
> >> >gcfg_objects.xml
> >> >
> >> >Has to be moved to an appropriate directory. Cannot be placed with the
> log
> >> >service. Talk to Anders W.
> >> >Maybe other reviewers also have comments about this.
> >> >
> >> [Vu] Yes, I think so. Anders & Mathi may give comments on this.
> >>
> >> >
> >> >---
> >> >lgs_imm_gcfg.cc
> >> >I hope you have thoroughly checked this code. This code is a rather
> quickly
> >> >created prototype code, is not very well tested and had not been
> reviewed
> >> by
> >> >anybody before you got it. It should also be verified with valgrind thread
> >> check
> >> >(has not been done)
> >> >- Spelling error on line 60. Replace "snd" with "and"
> >> >- There is no information telling what this function is doing and why
> >> >send_command() function
> >> [Vu] Thanks. I added the description on top of this function.
> >> >
> >> >
> >> >---
> >> >lgs_mbcsv.cc
> >> >Remove TODO on line 1625
> >> >Why adding inparameter nodeId to function
> dec_write_log_async_msg()?
> >> Not
> >> >used in function
> >> >Can also be removed from mds_dec() 

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Anders Widell]

See my comments inline.

regards,
Anders Widell

On 03/08/2016 03:49 AM, Vu Minh Nguyen wrote:
> Hi Lennart,
>
> Please see my responses inline, with [Vu].
>
> Regards, Vu.
>
>> -Original Message-
>> From: Lennart Lund [mailto:lennart.l...@ericsson.com]
>> Sent: Thursday, March 03, 2016 10:38 PM
>> To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
>> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> Garcia
>> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
> about
>> origin of log record [#1480]
>>
>> Hi Vu
>>
>> My comments:
>>
>> ---
>> logtest.c
>> get_attr_value()
>> This function looks like it can get a value from any attribute in any
> object but
>> this does not seems to be the case.
>> It can only get values from some specific objects and also not for all
> attributes
>> in those objects. This is very unclean code and should be improved. Also
> the
>> test code should be kept as clean as possible especially global functions.
>> There are several ways of handling this e.g.
>> A function that takes the object name and the attribute name (as a C
> strings)
>> as in parameters and has a void pointer for the fetched value. Since no
>> attributes in the log service can be multivalue it should be ok to always
> give
>> value[0] as output. This is also a void pointer. The calling function
> should know
>> how to typecast the fetched value to the correct type. The only limitation
> for
>> this function is that it will only return the first value in case of
> multivalue but
>> this should be no problem.
>>
> [Vu] Thanks. I propose to improve that in an enhancement ticket.
> With that ticket, we can add points to be improved.
>
>> ---
>> gcfg_classes.xml
>> gcfg_objects.xml
>>
>> Has to be moved to an appropriate directory. Cannot be placed with the log
>> service. Talk to Anders W.
>> Maybe other reviewers also have comments about this.
>>
> [Vu] Yes, I think so. Anders & Mathi may give comments on this.
[AndersW] We have one existing service that could be a candidate for 
owning these classes: NID. So one idea is to put them in the 
services/infrastructure/nid/config/ directory. Another alternative is to 
create a new config directory at the global scope: either osaf/config/ 
or osaf/services/config/. I don't have any strong preference for any of 
these alternatives, but if I must pick one I think it would be 
osaf/services/config/. Mathi, do you have any preference or other 
suggestion?

>
>> ---
>> lgs_imm_gcfg.cc
>> I hope you have thoroughly checked this code. This code is a rather quickly
>> created prototype code, is not very well tested and had not been reviewed
> by
>> anybody before you got it. It should also be verified with valgrind thread
> check
>> (has not been done)
>> - Spelling error on line 60. Replace "snd" with "and"
>> - There is no information telling what this function is doing and why
>> send_command() function
> [Vu] Thanks. I added the description on top of this function.
>>
>> ---
>> lgs_mbcsv.cc
>> Remove TODO on line 1625
>> Why adding inparameter nodeId to function dec_write_log_async_msg()? Not
>> used in function
>> Can also be removed from mds_dec() function? (line 799)
> [Vu] I removed them.
>>
>> ---
>> README
>> I suggest some changes in the text added to README
>>
>> Original text:
>> 3. New tokens are added (#1480)
>> ---
>> - @Cp: for showing the network name
>> - @Cq: for showing node name where the log record comes from.
>>
>> a) The network name comes from an configurable attribute
>> `opensafNetworkName`
>>which belongs to global configurable class `OpensafConfig`.
>>The attribute can be accessed via DN
>> `opensafConfigId=opensafGlobalConfig,safApp=OpenSAF`.
>>
>>LOG service is an applier to this object class, so that whenever there
> is
>> change in
>>network name attribute `opensafNetworkName`, LOG service will be
> notified.
>> b) Regarding node name, LOG service gets this information when decoding
>> messages at MDS layer.
>>
>> Suggested modifications:
>> 3. New tokens are added (#1480)
>> ---
>> - @Cp: for showing the network name
>> - @Cq: for showing node name where the log record comes from.
>>
>> a) The network name comes from an attribute, `opensafNetworkName`
>>which belongs to the `OpensafConfig` class.
>>The `opensafConfigId=opensafGlobalConfig,safApp=OpenSAF` object of this
>> class is an
>>OpenSAF global configuration object.
>>
>>LOG service is an applier for this object, so that whenever there is
> change of
>>network name attribute `opensafNetworkName`, LOG service will be
> notified.
>> b) Regarding node name, LOG service gets this information when decoding
>> messages at MDS layer.
>>
> [Vu] Thanks for your 

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Lennart Lund]

Hi Vu

Ack with comment:

I think you should fix the get_attr_value() function before pushing.
What is the decision for  placement of gcfg_classes.xml and gcfg_objects.xml? 
Must of course also be fixed 

Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 8 mars 2016 03:49
> To: Lennart Lund; Anders Widell; mathi.naic...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> Garcia
> Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information about
> origin of log record [#1480]
> 
> Hi Lennart,
> 
> Please see my responses inline, with [Vu].
> 
> Regards, Vu.
> 
> >-Original Message-
> >From: Lennart Lund [mailto:lennart.l...@ericsson.com]
> >Sent: Thursday, March 03, 2016 10:38 PM
> >To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
> >Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
> Garcia
> >Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
> about
> >origin of log record [#1480]
> >
> >Hi Vu
> >
> >My comments:
> >
> >---
> >logtest.c
> >get_attr_value()
> >This function looks like it can get a value from any attribute in any
> object but
> >this does not seems to be the case.
> >It can only get values from some specific objects and also not for all
> attributes
> >in those objects. This is very unclean code and should be improved. Also
> the
> >test code should be kept as clean as possible especially global functions.
> >There are several ways of handling this e.g.
> >A function that takes the object name and the attribute name (as a C
> strings)
> >as in parameters and has a void pointer for the fetched value. Since no
> >attributes in the log service can be multivalue it should be ok to always
> give
> >value[0] as output. This is also a void pointer. The calling function
> should know
> >how to typecast the fetched value to the correct type. The only limitation
> for
> >this function is that it will only return the first value in case of
> multivalue but
> >this should be no problem.
> >
> [Vu] Thanks. I propose to improve that in an enhancement ticket.
> With that ticket, we can add points to be improved.
> 
> >
> >---
> >gcfg_classes.xml
> >gcfg_objects.xml
> >
> >Has to be moved to an appropriate directory. Cannot be placed with the log
> >service. Talk to Anders W.
> >Maybe other reviewers also have comments about this.
> >
> [Vu] Yes, I think so. Anders & Mathi may give comments on this.
> 
> >
> >---
> >lgs_imm_gcfg.cc
> >I hope you have thoroughly checked this code. This code is a rather quickly
> >created prototype code, is not very well tested and had not been reviewed
> by
> >anybody before you got it. It should also be verified with valgrind thread
> check
> >(has not been done)
> >- Spelling error on line 60. Replace "snd" with "and"
> >- There is no information telling what this function is doing and why
> >send_command() function
> [Vu] Thanks. I added the description on top of this function.
> >
> >
> >---
> >lgs_mbcsv.cc
> >Remove TODO on line 1625
> >Why adding inparameter nodeId to function dec_write_log_async_msg()?
> Not
> >used in function
> >Can also be removed from mds_dec() function? (line 799)
> [Vu] I removed them.
> >
> >
> >---
> >README
> >I suggest some changes in the text added to README
> >
> >Original text:
> >3. New tokens are added (#1480)
> >---
> >- @Cp: for showing the network name
> >- @Cq: for showing node name where the log record comes from.
> >
> >a) The network name comes from an configurable attribute
> >`opensafNetworkName`
> >   which belongs to global configurable class `OpensafConfig`.
> >   The attribute can be accessed via DN
> >`opensafConfigId=opensafGlobalConfig,safApp=OpenSAF`.
> >
> >   LOG service is an applier to this object class, so that whenever there
> is
> >change in
> >   network name attribute `opensafNetworkName`, LOG service will be
> notified.
> >
> >b) Regarding node name, LOG service gets this information when decoding
> >messages at MDS layer.
> >
> >Suggested modifications:
> >3. New tokens are added (#1480)
> >---
> >- @Cp: for showing the network name
> >- @Cq: for showing node name where the log record comes from.
> >
> >a) The network name comes from an attribute, `opensafNetworkName`
> >   which belongs to the `OpensafConfig` class.
> >   The `opensafConfigId=opensafGlobalConfig,safApp=OpenSAF` object of
> this
> >class is an
> >   OpenSAF global configuration object.
> >
> >   LOG service is an applier for this object, so that whenever there is
> change of
> >   network name attribute `opensafNetworkName`, LOG service will be
> notified.
> >
> >b) Regarding node name, LOG service gets this information when decoding
> 

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Vu Minh Nguyen]

Hi Mathi,

Have you had time to look at this yet? 

Regards, Vu.

>-Original Message-
>From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
>Sent: Friday, February 26, 2016 8:49 AM
>To: anders.wid...@ericsson.com; lennart.l...@ericsson.com;
>mathi.naic...@oracle.com
>Cc: opensaf-devel@lists.sourceforge.net
>Subject: [devel] [PATCH 0 of 1] Review Request for log: Extend information
>about origin of log record [#1480]
>
>Summary: log: Extend information about origin of log record [#1480]
>Review request for Trac Ticket(s): #1468
>Peer Reviewer(s): Lennart, Anders W, Mathi
>Pull request to: Lennart
>Affected branch(es): Default
>Development branch: Default
>
>
>Impacted area   Impact y/n
>
> Docsn
> Build systemn
> RPM/packaging   n
> Configuration files n
> Startup scripts n
> SAF servicesy
> OpenSAF servicesn
> Core libraries  n
> Samples n
> Tests   n
> Other   n
>
>
>Comments (indicate scope for each "y" above):
>-
> <>
>
>changeset a6c0e7e9785c75c6dcf57404027fc92fc572a25f
>Author:Vu Minh Nguyen 
>Date:  Tue, 02 Feb 2016 10:02:37 +0700
>
>   log: Extend information about origin of log record [#1480]
>
>   Add new tokens (@Cq and @Cp) to represent node name and network
>name.
>
>
>Added Files:
>
> osaf/services/saf/logsv/config/gcfg_classes.xml
> osaf/services/saf/logsv/config/gcfg_objects.xml
> osaf/services/saf/logsv/lgs/lgs_imm_gcfg.cc
> osaf/services/saf/logsv/lgs/lgs_imm_gcfg.h
>
>
>Complete diffstat:
>--
> osaf/services/saf/logsv/README  |16 +
> osaf/services/saf/logsv/config/Makefile.am  | 4 +-
> osaf/services/saf/logsv/config/gcfg_classes.xml |18 +
> osaf/services/saf/logsv/config/gcfg_objects.xml | 6 +
> osaf/services/saf/logsv/lgs/Makefile.am | 6 +-
> osaf/services/saf/logsv/lgs/lgs.h   | 1 +
> osaf/services/saf/logsv/lgs/lgs_amf.cc  | 9 +-
> osaf/services/saf/logsv/lgs/lgs_evt.cc  |11 +-
> osaf/services/saf/logsv/lgs/lgs_evt.h   | 1 +
> osaf/services/saf/logsv/lgs/lgs_fmt.cc  |52 -
> osaf/services/saf/logsv/lgs/lgs_fmt.h   |20 +-
> osaf/services/saf/logsv/lgs/lgs_imm_gcfg.cc |  1082
>++
>+
> osaf/services/saf/logsv/lgs/lgs_imm_gcfg.h  |28 ++
> osaf/services/saf/logsv/lgs/lgs_main.cc | 1 +
> osaf/services/saf/logsv/lgs/lgs_mbcsv.cc|50 +++-
> osaf/services/saf/logsv/lgs/lgs_mbcsv.h | 1 +
> osaf/services/saf/logsv/lgs/lgs_mds.cc  |13 +-
> tests/logsv/logtest.c   | 7 +
> tests/logsv/logtest.h   | 2 +
> tests/logsv/tet_LogOiOps.c  |   205

> 20 files changed, 1498 insertions(+), 35 deletions(-)
>
>
>Testing Commands:
>-
> There are 02 added new test cases to test node name token
> and network name token. Run them (following) for testing.
>
> logtest 4 63
> logtest 4 64
>
>NOTE (dependencies):
>
>This patch has to be merged on top of following patches (in review/not
pushed
>yet)
>1) #1522 MDS: Include node name as a part of control events
>2) #1179 log: add support for cloud resilience feature
>
>
>Testing, Expected Results:
>--
> All test cases passed
>
>
>Conditions of Submission:
>-
> Get ack from peer reviewers
>
>
>Arch  Built StartedLinux distro
>---
>mipsn  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. 

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Vu Minh Nguyen]

Hi Lennart,

Please see my responses inline, with [Vu].

Regards, Vu.

>-Original Message-
>From: Lennart Lund [mailto:lennart.l...@ericsson.com]
>Sent: Thursday, March 03, 2016 10:38 PM
>To: Vu Minh Nguyen; Anders Widell; mathi.naic...@oracle.com
>Cc: opensaf-devel@lists.sourceforge.net; Beatriz Brandao; Jorge Pacheco
Garcia
>Subject: RE: [PATCH 0 of 1] Review Request for log: Extend information
about
>origin of log record [#1480]
>
>Hi Vu
>
>My comments:
>
>---
>logtest.c
>get_attr_value()
>This function looks like it can get a value from any attribute in any
object but
>this does not seems to be the case.
>It can only get values from some specific objects and also not for all
attributes
>in those objects. This is very unclean code and should be improved. Also
the
>test code should be kept as clean as possible especially global functions.
>There are several ways of handling this e.g.
>A function that takes the object name and the attribute name (as a C
strings)
>as in parameters and has a void pointer for the fetched value. Since no
>attributes in the log service can be multivalue it should be ok to always
give
>value[0] as output. This is also a void pointer. The calling function
should know
>how to typecast the fetched value to the correct type. The only limitation
for
>this function is that it will only return the first value in case of
multivalue but
>this should be no problem.
>
[Vu] Thanks. I propose to improve that in an enhancement ticket.
With that ticket, we can add points to be improved.

>
>---
>gcfg_classes.xml
>gcfg_objects.xml
>
>Has to be moved to an appropriate directory. Cannot be placed with the log
>service. Talk to Anders W.
>Maybe other reviewers also have comments about this.
>
[Vu] Yes, I think so. Anders & Mathi may give comments on this.

>
>---
>lgs_imm_gcfg.cc
>I hope you have thoroughly checked this code. This code is a rather quickly
>created prototype code, is not very well tested and had not been reviewed
by
>anybody before you got it. It should also be verified with valgrind thread
check
>(has not been done)
>- Spelling error on line 60. Replace "snd" with "and"
>- There is no information telling what this function is doing and why
>send_command() function
[Vu] Thanks. I added the description on top of this function.
>
>
>---
>lgs_mbcsv.cc
>Remove TODO on line 1625
>Why adding inparameter nodeId to function dec_write_log_async_msg()? Not
>used in function
>Can also be removed from mds_dec() function? (line 799)
[Vu] I removed them.
>
>
>---
>README
>I suggest some changes in the text added to README
>
>Original text:
>3. New tokens are added (#1480)
>---
>- @Cp: for showing the network name
>- @Cq: for showing node name where the log record comes from.
>
>a) The network name comes from an configurable attribute
>`opensafNetworkName`
>   which belongs to global configurable class `OpensafConfig`.
>   The attribute can be accessed via DN
>`opensafConfigId=opensafGlobalConfig,safApp=OpenSAF`.
>
>   LOG service is an applier to this object class, so that whenever there
is
>change in
>   network name attribute `opensafNetworkName`, LOG service will be
notified.
>
>b) Regarding node name, LOG service gets this information when decoding
>messages at MDS layer.
>
>Suggested modifications:
>3. New tokens are added (#1480)
>---
>- @Cp: for showing the network name
>- @Cq: for showing node name where the log record comes from.
>
>a) The network name comes from an attribute, `opensafNetworkName`
>   which belongs to the `OpensafConfig` class.
>   The `opensafConfigId=opensafGlobalConfig,safApp=OpenSAF` object of this
>class is an
>   OpenSAF global configuration object.
>
>   LOG service is an applier for this object, so that whenever there is
change of
>   network name attribute `opensafNetworkName`, LOG service will be
notified.
>
>b) Regarding node name, LOG service gets this information when decoding
>messages at MDS layer.
>
[Vu] Thanks for your suggestion. I will update README file accordingly.
>
>Thanks
>Lennart
>
>> -Original Message-
>> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
>> Sent: den 26 februari 2016 02:49
>> To: Anders Widell; Lennart Lund; mathi.naic...@oracle.com
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: [PATCH 0 of 1] Review Request for log: Extend information about
>> origin of log record [#1480]
>>
>> Summary: log: Extend information about origin of log record [#1480]
>> Review request for Trac Ticket(s): #1468
>> Peer Reviewer(s): Lennart, Anders W, Mathi
>> Pull request to: Lennart
>> Affected branch(es): Default
>> Development branch: Default
>>
>> 
>> Impacted area   Impact y/n
>> 
>>  Docsn
>>  

Re: [devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Lennart Lund]

Hi Vu

My comments:

---
logtest.c
get_attr_value()
This function looks like it can get a value from any attribute in any object 
but this does not seems to be the case.
It can only get values from some specific objects and also not for all 
attributes in those objects. This is very unclean code and should be improved. 
Also the test code should be kept as clean as possible especially global 
functions.
There are several ways of handling this e.g.
A function that takes the object name and the attribute name (as a C strings) 
as in parameters and has a void pointer for the fetched value. Since no 
attributes in the log service can be multivalue it should be ok to always give 
value[0] as output. This is also a void pointer. The calling function should 
know how to typecast the fetched value to the correct type. The only limitation 
for this function is that it will only return the first value in case of 
multivalue but this should be no problem.


---
gcfg_classes.xml 
gcfg_objects.xml

Has to be moved to an appropriate directory. Cannot be placed with the log 
service. Talk to Anders W.
Maybe other reviewers also have comments about this.


---
lgs_imm_gcfg.cc
I hope you have thoroughly checked this code. This code is a rather quickly 
created prototype code, is not very well tested and had not been reviewed by 
anybody before you got it. It should also be verified with valgrind thread 
check (has not been done)
- Spelling error on line 60. Replace "snd" with "and"
- There is no information telling what this function is doing and why 
send_command() function


---
lgs_mbcsv.cc
Remove TODO on line 1625
Why adding inparameter nodeId to function dec_write_log_async_msg()? Not used 
in function
Can also be removed from mds_dec() function? (line 799)


---
README
I suggest some changes in the text added to README

Original text:
3. New tokens are added (#1480)
---
- @Cp: for showing the network name
- @Cq: for showing node name where the log record comes from.

a) The network name comes from an configurable attribute `opensafNetworkName`
   which belongs to global configurable class `OpensafConfig`.
   The attribute can be accessed via DN 
`opensafConfigId=opensafGlobalConfig,safApp=OpenSAF`.

   LOG service is an applier to this object class, so that whenever there is 
change in
   network name attribute `opensafNetworkName`, LOG service will be notified.

b) Regarding node name, LOG service gets this information when decoding 
messages at MDS layer.

Suggested modifications:
3. New tokens are added (#1480)
---
- @Cp: for showing the network name
- @Cq: for showing node name where the log record comes from.

a) The network name comes from an attribute, `opensafNetworkName`
   which belongs to the `OpensafConfig` class.
   The `opensafConfigId=opensafGlobalConfig,safApp=OpenSAF` object of this 
class is an
   OpenSAF global configuration object.

   LOG service is an applier for this object, so that whenever there is change 
of
   network name attribute `opensafNetworkName`, LOG service will be notified.

b) Regarding node name, LOG service gets this information when decoding 
messages at MDS layer.


Thanks
Lennart

> -Original Message-
> From: Vu Minh Nguyen [mailto:vu.m.ngu...@dektech.com.au]
> Sent: den 26 februari 2016 02:49
> To: Anders Widell; Lennart Lund; mathi.naic...@oracle.com
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 0 of 1] Review Request for log: Extend information about
> origin of log record [#1480]
> 
> Summary: log: Extend information about origin of log record [#1480]
> Review request for Trac Ticket(s): #1468
> Peer Reviewer(s): Lennart, Anders W, Mathi
> Pull request to: Lennart
> Affected branch(es): Default
> Development branch: Default
> 
> 
> Impacted area   Impact y/n
> 
>  Docsn
>  Build systemn
>  RPM/packaging   n
>  Configuration files n
>  Startup scripts n
>  SAF servicesy
>  OpenSAF servicesn
>  Core libraries  n
>  Samples n
>  Tests   n
>  Other   n
> 
> 
> Comments (indicate scope for each "y" above):
> -
>  <>
> 
> changeset a6c0e7e9785c75c6dcf57404027fc92fc572a25f
> Author:   Vu Minh Nguyen 
> Date: Tue, 02 Feb 2016 10:02:37 +0700
> 
>   log: Extend information about origin of log record [#1480]
> 
>   Add new tokens (@Cq and @Cp) to represent node name and
> network name.
> 
> 
> Added Files:
> 
>  osaf/services/saf/logsv/config/gcfg_classes.xml
>  osaf/services/saf/logsv/config/gcfg_objects.xml
>  

[devel] [PATCH 0 of 1] Review Request for log: Extend information about origin of log record [#1480]

[Vu Minh Nguyen]

Summary: log: Extend information about origin of log record [#1480]
Review request for Trac Ticket(s): #1468
Peer Reviewer(s): Lennart, Anders W, Mathi
Pull request to: Lennart
Affected branch(es): Default
Development branch: Default


Impacted area   Impact y/n

 Docsn
 Build systemn
 RPM/packaging   n
 Configuration files n
 Startup scripts n
 SAF servicesy
 OpenSAF servicesn
 Core libraries  n
 Samples n
 Tests   n
 Other   n


Comments (indicate scope for each "y" above):
-
 <>

changeset a6c0e7e9785c75c6dcf57404027fc92fc572a25f
Author: Vu Minh Nguyen 
Date:   Tue, 02 Feb 2016 10:02:37 +0700

log: Extend information about origin of log record [#1480]

Add new tokens (@Cq and @Cp) to represent node name and network name.


Added Files:

 osaf/services/saf/logsv/config/gcfg_classes.xml
 osaf/services/saf/logsv/config/gcfg_objects.xml
 osaf/services/saf/logsv/lgs/lgs_imm_gcfg.cc
 osaf/services/saf/logsv/lgs/lgs_imm_gcfg.h


Complete diffstat:
--
 osaf/services/saf/logsv/README  |16 +
 osaf/services/saf/logsv/config/Makefile.am  | 4 +-
 osaf/services/saf/logsv/config/gcfg_classes.xml |18 +
 osaf/services/saf/logsv/config/gcfg_objects.xml | 6 +
 osaf/services/saf/logsv/lgs/Makefile.am | 6 +-
 osaf/services/saf/logsv/lgs/lgs.h   | 1 +
 osaf/services/saf/logsv/lgs/lgs_amf.cc  | 9 +-
 osaf/services/saf/logsv/lgs/lgs_evt.cc  |11 +-
 osaf/services/saf/logsv/lgs/lgs_evt.h   | 1 +
 osaf/services/saf/logsv/lgs/lgs_fmt.cc  |52 -
 osaf/services/saf/logsv/lgs/lgs_fmt.h   |20 +-
 osaf/services/saf/logsv/lgs/lgs_imm_gcfg.cc |  1082 
+++
 osaf/services/saf/logsv/lgs/lgs_imm_gcfg.h  |28 ++
 osaf/services/saf/logsv/lgs/lgs_main.cc | 1 +
 osaf/services/saf/logsv/lgs/lgs_mbcsv.cc|50 +++-
 osaf/services/saf/logsv/lgs/lgs_mbcsv.h | 1 +
 osaf/services/saf/logsv/lgs/lgs_mds.cc  |13 +-
 tests/logsv/logtest.c   | 7 +
 tests/logsv/logtest.h   | 2 +
 tests/logsv/tet_LogOiOps.c  |   205 
 20 files changed, 1498 insertions(+), 35 deletions(-)


Testing Commands:
-
 There are 02 added new test cases to test node name token
 and network name token. Run them (following) for testing.

 logtest 4 63
 logtest 4 64

NOTE (dependencies): 

This patch has to be merged on top of following patches (in review/not pushed 
yet)
1) #1522 MDS: Include node name as a part of control events 
2) #1179 log: add support for cloud resilience feature


Testing, Expected Results:
--
 All test cases passed


Conditions of Submission:
-
 Get ack from peer reviewers


Arch  Built StartedLinux distro
---
mipsn  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