[checkmk-commits] 5409 FIX Windows eventlog: wrong last state saved

Module: check_mk Branch: master Commit: 33281d4ab0f81e1850669c0d9624aefcd610f3ba URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=33281d4ab0f81e… Author: Jukka Aro &lt;ja(a)mathias-kettner.de&gt; Date: Mon Oct 30 16:20:38 2017 +0100 5409 FIX Windows eventlog: wrong last state saved The initial run of Windows agent after fresh installation caused an incorrect last state to be written in the state file eventstate.txt. As a result, no new entries were read from the eventlog. Broken since commit 9619c43a171b7e9258f012f547c1d186660c6268. Change-Id: I47378f667b7ecf8c481f7ed9c7105623b4051a2b --- .werks/5409 | 16 ++++++++++++++++ agents/windows/EventLog.cc | 24 ++++++++++++++++++------ agents/windows/EventLog.h | 7 ++++++- agents/windows/EventLogVista.cc | 23 ++++++++++++++++++++--- agents/windows/EventLogVista.h | 4 +++- agents/windows/IEventLog.h | 19 ++++++++++++++----- agents/windows/build_version | 2 +- agents/windows/sections/SectionEventlog.cc | 10 ++++++++-- 8 files changed, 86 insertions(+), 19 deletions(-) diff --git a/.werks/5409 b/.werks/5409 new file mode 100644 index 0000000..39cdf54 --- /dev/null +++ b/.werks/5409 @@ -0,0 +1,16 @@ +Title: Windows eventlog: wrong last state saved +Level: 1 +Component: checks +Class: fix +Compatible: compat +Edition: cre +State: unknown +Version: 1.5.0i1 +Date: 1509376514 + +The initial run of Windows agent after fresh installation caused an +incorrect last state to be written in the state file eventstate.txt. +As a result, no new entries were read from the eventlog. Broken since +commit 91797ac9f7d69fc94119c08ea0df4baccaea2b6. + + diff --git a/agents/windows/EventLog.cc b/agents/windows/EventLog.cc index a840d24..8fb0d50 100644 --- a/agents/windows/EventLog.cc +++ b/agents/windows/EventLog.cc @@ -297,23 +297,23 @@ void EventLog::reset() { wstring EventLog::getName() const { return _name; } -uint64_t EventLog::seek(uint64_t record_number) { +void EventLog::seek(uint64_t record_number) { DWORD oldest_record, record_count; if (_log.GetOldestEventLogRecord(&oldest_record) && (record_number < oldest_record)) { - // can't seek to older record + // Beyond the oldest record: _record_offset = oldest_record; } else if (_log.GetNumberOfEventLogRecords(&record_count) && - (record_number > oldest_record + record_count)) { + (record_number >= oldest_record + record_count)) { + // Beyond the newest record. Note: set offset intentionally to the next + // record after the currently last one! _record_offset = oldest_record + record_count; } else { - // not actually seeking but storing for the next actual read + // Within bounds, the offset for the next actual read: _record_offset = record_number; } _buffer_offset = _buffer_used; // enforce that a new chunk is fetched - - return _record_offset; } std::unique_ptr<IEventLogRecord> EventLog::read() { @@ -351,6 +351,18 @@ std::unique_ptr<IEventLogRecord> EventLog::read() { } } +uint64_t EventLog::getLastRecordId() { + DWORD oldestRecord = 0; + DWORD recordCount = 0; + if (_log.GetOldestEventLogRecord(&oldestRecord) && + _log.GetNumberOfEventLogRecords(&recordCount) && + oldestRecord + recordCount > 0) { + return oldestRecord + recordCount - 1; + } else { + return 0; + } +} + bool EventLog::fillBuffer() { _buffer_offset = 0; diff --git a/agents/windows/EventLog.h b/agents/windows/EventLog.h index eb063fc..cab226b 100644 --- a/agents/windows/EventLog.h +++ b/agents/windows/EventLog.h @@ -149,7 +149,7 @@ public: * In this case this function will still work as expected but the next read * will be slow. */ - virtual uint64_t seek(uint64_t record_id) override; + virtual void seek(uint64_t record_id) override; /** * read the next eventlog record @@ -160,6 +160,11 @@ public: virtual std::unique_ptr<IEventLogRecord> read() override; /** + * return the ID of the last record in eventlog + */ + virtual uint64_t getLastRecordId() override; + + /** * get a list of dlls that contain eventid->message mappings for this * eventlog and the specified source */ diff --git a/agents/windows/EventLogVista.cc b/agents/windows/EventLogVista.cc index 1b73e1a..b93bada 100644 --- a/agents/windows/EventLogVista.cc +++ b/agents/windows/EventLogVista.cc @@ -338,7 +338,7 @@ std::wstring EventLogVista::renderBookmark(EVT_HANDLE bookmark) const { return buffer; } -uint64_t EventLogVista::seek(uint64_t record_id) { +void EventLogVista::seek(uint64_t record_id) { { // The api to retrieve the oldest event log id is bugged. bloody hell... // to get the right offset if record_id is beyond the valid range, we @@ -410,8 +410,6 @@ uint64_t EventLogVista::seek(uint64_t record_id) { throw win_exception( _winapi, std::string("failed to subscribe to ") + to_utf8(_path)); } - - return record_id; } std::unique_ptr<IEventLogRecord> EventLogVista::read() { @@ -425,6 +423,25 @@ std::unique_ptr<IEventLogRecord> EventLogVista::read() { _winapi); } +uint64_t EventLogVista::getLastRecordId() { + auto log = std::make_unique<EventLogWrapper>( + *_evt, EvtQueryReverseDirection, _path, _winapi); + + EVT_HANDLE event_handle = nullptr; + DWORD num_events = 0; + if (evt().next && + evt().next(log->get_handle(), 1, &event_handle, INFINITE, 0, + &num_events)) { + auto event = std::make_unique<ManagedEventHandle>(*_evt, event_handle); + + return EventLogRecordVista(event->get_handle(), _evt.get(), + _render_context->get_handle(), _winapi) + .recordId(); + } else { + return 0; + } +} + bool EventLogVista::fillBuffer() { // this ensures all previous event handles are closed and nulled reset(); diff --git a/agents/windows/EventLogVista.h b/agents/windows/EventLogVista.h index de9195f..bbea22f 100644 --- a/agents/windows/EventLogVista.h +++ b/agents/windows/EventLogVista.h @@ -150,10 +150,12 @@ public: virtual void reset() override; - virtual uint64_t seek(uint64_t record_id) override; + virtual void seek(uint64_t record_id) override; virtual std::unique_ptr<IEventLogRecord> read() override; + virtual uint64_t getLastRecordId() override; + private: static const int EVENT_BLOCK_SIZE = 16; diff --git a/agents/windows/IEventLog.h b/agents/windows/IEventLog.h index 10d7df0..ebeb25c 100644 --- a/agents/windows/IEventLog.h +++ b/agents/windows/IEventLog.h @@ -70,13 +70,17 @@ public: * seek to the specified record on the next read or, if the record_number is * older than the oldest existing record, seek to the beginning. If the * record_number is the highest representable uint32_t, seek to the end of - * the log such that only future events are retrieveda + * the log such that only future events are retrieved * - * returns the actual record_id we seeked to, which may differ from the - * input - * if it was outside the available range + * WARNING: + * The implementations for pre-Vista and post-Vista are completely + * different. + * We *must not* return any value as it is different between pre/post Vista. + * For obtaining the ID of the last record in eventlog, please use + * getLastRecordId instead. It has own implementations for pre/post Vista + * but return a uniformly correct value. */ - virtual uint64_t seek(uint64_t record_id) = 0; + virtual void seek(uint64_t record_id) = 0; /** * read the next eventlog record @@ -87,6 +91,11 @@ public: virtual std::unique_ptr<IEventLogRecord> read() = 0; /** + * return the ID of the last record in eventlog + */ + virtual uint64_t getLastRecordId() = 0; + + /** * get a list of dlls that contain eventid->message mappings for this * eventlog and the specified source */ diff --git a/agents/windows/build_version b/agents/windows/build_version index ce89b3e..47efd9a 100644 --- a/agents/windows/build_version +++ b/agents/windows/build_version @@ -1 +1 @@ -2992 +2994 diff --git a/agents/windows/sections/SectionEventlog.cc b/agents/windows/sections/SectionEventlog.cc index 9e6f79d..773e3d4 100644 --- a/agents/windows/sections/SectionEventlog.cc +++ b/agents/windows/sections/SectionEventlog.cc @@ -166,6 +166,9 @@ std::pair<uint64_t, int> processEventLog( previouslyReadId + (uint64limits::max() == previouslyReadId ? 0 : 1); int worstState = 0; uint64_t lastRecordId = previouslyReadId; + // WARNING: + // seek implementations for pre-Vista and post-Vista are completely different. + // seek *must not* return any value as it is different between pre/post Vista. log.seek(seekPosition); while (auto record = std::move(log.read())) { lastRecordId = record->recordId(); @@ -210,8 +213,11 @@ uint64_t SectionEventlog::outputEventlog(std::ostream &out, const char *logname, }; processEventLog(*log, previouslyReadId, level, outputRecord); } - - return lastReadId; + // Return the last entry number. We need to fetch the last record ID + // separately if INT_MAX was used as seek offset and no new entries + // were read. + return (std::numeric_limits<uint64_t>::max() == lastReadId) ? + log->getLastRecordId() : lastReadId; } } catch (const std::exception &e) { Error(_logger) << "failed to read event log: " << e.what() << std::endl;