This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/837/

On September 20th, 2013, 12:56 p.m. UTC, Ales Ledvinka wrote:

src/journald/LMI_JournalMessageLogProvider.c (Diff revision 2)
33
#define CIM_MESSAGELOG_ITERATOR_RESULT_SUCCESS 0
This should map to the konkret's "OK" code rather then to exact number.

If you disagree then see the PositionAtRecord method comment regarding returning of result.
As discussed face-to-face, this point should about CMPIStatus, not the result.

On September 20th, 2013, 12:56 p.m. UTC, Ales Ledvinka wrote:

src/journald/LMI_JournalMessageLogProvider.c (Diff revision 2)
49
#if 0
remove or is this future work?
Removed the block and added comment for journal_iters_destroy(). Leaving cleanup on CIMOM itself.

On September 20th, 2013, 12:56 p.m. UTC, Ales Ledvinka wrote:

src/journald/LMI_JournalMessageLogProvider.c (Diff revision 2)
KUint32 LMI_JournalMessageLog_PositionAtRecord(
236
    KSetStatus(status, ERR_NOT_SUPPORTED);
260
    if (IterationIdentifier->null || ! IterationIdentifier->exists ||
duverujem cimmomu/konkretu za kazdych okolnosti nebo radsi pridat:
NULL == IterationIdentifier || ...

NULL == MoveAbsolute

NULL == RecordNumber

...
This is a konkret's job. If it fails to provide consistent arguments, we should fix it there at a first place. 

On September 20th, 2013, 12:56 p.m. UTC, Ales Ledvinka wrote:

src/journald/LMI_JournalMessageLogProvider.c (Diff revision 2)
KUint32 LMI_JournalMessageLog_PositionAtRecord(
264
        KUint32_Set(&result, CIM_MESSAGELOG_ITERATOR_RESULT_FAILED);
This is useless since the status is not konkret's ok. See instead of filenameProvider.c see the filename.h the _Dispatch_Method() for more details on result processing of invoked method.
Fixed, revised status values completely. To be clear, result values (mapping to CMPIResult in konkret-generated code) stay the same as this is what clients get as a method call return value. Only CMPIStatus have been changed to reflect method call status.

On September 20th, 2013, 12:56 p.m. UTC, Ales Ledvinka wrote:

src/journald/LMI_JournalMessageLogProvider.c (Diff revision 2)
KUint32 LMI_JournalMessageLog_PositionAtRecord(
277
        KUint32_Set(&result, CIM_MESSAGELOG_ITERATOR_RESULT_FAILED);
missing status or status from previous operation. depending on the operation status if it's other then KOkay(status) match the line returning result might be useless depending on the status code returned
Again, this was meant for status, not the result.

- Tomáš


On September 20th, 2013, 10:05 a.m. UTC, Tomáš Bžatek wrote:

Review request for OpenLMI Developers.
By Tomáš Bžatek.

Updated Sept. 20, 2013, 10:05 a.m.

Repository: openlmi-providers

Description

    journald: Add support for iterators
    
    This implements CIM_MessageLog iterator functionality according to
    the model. The only limitation, or, let's say shortcoming, is a
    free-form output data format from the GetRecord call. Currently we
    feed the uint8 array with UTF-8 chars and it's up to the client to
    deal with it (as suggested in the model). This may be subject to
    change.
    
    Many methods are returning modified iterator that has been passed
    in and that allows having persistent iterators. As long as CIMOMs
    may destroy the instances and unload providers on inactivity, any
    particular delay would cause loss of the iterator. The iterator
    string contains journal cursor string, uniquely identifying a
    record. When this part of the iterator string is valid, the
    provider is able to reopen journal and seek to that position,
    allowing seamless continuation of the itearation process. If the
    cursor points to non-existing record, an error is returned. This
    typically happens when journal is rotated or the cursor is very
    old.
    
    The provider keeps track of active iterators during its lifetime,
    keeping the journal open for fast access. Clients are supposed to
    close the iterator by calling the CancelIteration method.


--
some TODOs will be addressed in future patches
tests will be published soon

Diffs

  • src/journald/LMI_JournalMessageLogProvider.c (PRE-CREATION)
  • src/journald/instutil.h (PRE-CREATION)
  • src/journald/instutil.c (PRE-CREATION)

View Diff