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

On September 19th, 2013, 5:47 a.m. UTC, Jan Synacek wrote:

src/journald/LMI_JournalLogRecordProvider.c (Diff revision 1)
169
get_string_property(const char *property_name, const CMPIInstance* ci)
There is a very similar function in the logicalfile provider. Maybe we should move it to the libcommon.
Good idea, will do that in separate commits. Since my journald branch is not up-to-date and there were changes in master, I'll need to rebase and align my code first. But since there are several patches currently on review, let's do this as soon as they're finished.

On September 19th, 2013, 5:47 a.m. UTC, Jan Synacek wrote:

src/journald/LMI_JournalLogRecordProvider.c (Diff revision 1)
175
    if (rc.msg != NULL || (d.state & CMPI_nullValue) == CMPI_nullValue ||
My gut feeling here is that it's better to check for the rc than for the message.

if (rc.rc ...)
or more explicit and readable
ir (rc.rc != CMPI_RC_OK ...)
Right, an old habit from sblim provider code...

On September 19th, 2013, 5:47 a.m. UTC, Jan Synacek wrote:

src/journald/LMI_JournalLogRecordProvider.c (Diff revision 1)
226
    if (r < 0) {
227
        sd_journal_close(journal);
228
        KReturn2(_cb, ERR_FAILED, "Error seeking to the end of the journal: %s\n", strerror(-r));
229
    }
These pieces of code are fairly common throughout the code. Wrap in a macro / make a function?
Even though I'm not fan of local custom defines as it looks horribly disassociated to the rest of the code, I admit the current code looks horrible the same way. Moved to macro for the time being, until I decide it looks horrible again :-)

On September 19th, 2013, 5:47 a.m. UTC, Jan Synacek wrote:

s/fiull/full/ in the description.
Nice catch, thx.

- Tomáš


On September 18th, 2013, 3:22 p.m. UTC, Tomáš Bžatek wrote:

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

Updated Sept. 18, 2013, 3:22 p.m.

Repository: openlmi-providers

Description

    journald: Support writing new records
    
    This change adds ability to report new messages in the journal log.
    This is done by the LMI_JournalLogRecord.CreateInstance() method call
    taking only CreationClassName, LogCreationClassName, LogName and DataFormat
    key properties as mandatory, with no need to specify the MessageTimestamp and
    RecordID key properties. The returned instance will contain all the
    information with actual RecordID key property of the new record.
    
    As long as sd_journal_send() actually only queues the request, we need
    to watch for journal changes in order to find the new record. This is
    quite unfortunate as the daemon could be busy and we can't wait forever.
    Messages are available immediately usually, a timeout is set to 5 seconds
    for the moment.
    
    Record matching is done by comparing the message itself, the origin function
    name and PID. This brings sufficient amount of confidence though more
    sophisticated approach may be needed in future to ensure fiull uniqueness.

Diffs

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

View Diff