diff options
author | Matt Spinler <spinler@us.ibm.com> | 2019-07-17 13:54:30 -0500 |
---|---|---|
committer | Matt Spinler <spinler@us.ibm.com> | 2019-08-05 12:49:54 -0500 |
commit | 89fa082a68c975128db6fa96eab33200c6bc7bb6 (patch) | |
tree | 30ef8f010c8431536b631c47fa54bab43ecc2f51 | |
parent | cb6b059eec52ddfe099221de003d7dca86ce1db0 (diff) | |
download | phosphor-logging-89fa082a68c975128db6fa96eab33200c6bc7bb6.tar.gz phosphor-logging-89fa082a68c975128db6fa96eab33200c6bc7bb6.zip |
PEL: Add repository to save PELs
Create the Repository class that can save PELs in (and later retrieve
them from) the filesystem. It provides an add() method that can add
a PEL object to the repository.
Now, when the Manager class sees an OpenBMC event log created with the
RAWPEL metadata in the AdditionalData property that points at a file
that contains a PEL, it can save that PEL. Before the PEL is saved, the
log ID and commit timestamp fields in the PEL will be updated - the log
ID to a unique value, and the timestamp to the current time.
Change-Id: I8dbaddf0f155bcb6d40b933294ada83feb75ce53
-rw-r--r-- | extensions/openpower-pels/manager.cpp | 54 | ||||
-rw-r--r-- | extensions/openpower-pels/manager.hpp | 10 | ||||
-rw-r--r-- | extensions/openpower-pels/openpower-pels.mk | 1 | ||||
-rw-r--r-- | extensions/openpower-pels/paths.cpp | 7 | ||||
-rw-r--r-- | extensions/openpower-pels/paths.hpp | 5 | ||||
-rw-r--r-- | extensions/openpower-pels/repository.cpp | 71 | ||||
-rw-r--r-- | extensions/openpower-pels/repository.hpp | 62 | ||||
-rw-r--r-- | test/openpower-pels/Makefile.include | 23 | ||||
-rw-r--r-- | test/openpower-pels/paths.cpp | 13 | ||||
-rw-r--r-- | test/openpower-pels/pel_manager_test.cpp | 66 | ||||
-rw-r--r-- | test/openpower-pels/pel_utils.cpp | 2 | ||||
-rw-r--r-- | test/openpower-pels/pel_utils.hpp | 20 | ||||
-rw-r--r-- | test/openpower-pels/repository_test.cpp | 73 |
13 files changed, 406 insertions, 1 deletions
diff --git a/extensions/openpower-pels/manager.cpp b/extensions/openpower-pels/manager.cpp index 3e22336..a009188 100644 --- a/extensions/openpower-pels/manager.cpp +++ b/extensions/openpower-pels/manager.cpp @@ -1,6 +1,10 @@ #include "manager.hpp" #include "additional_data.hpp" +#include "pel.hpp" + +#include <filesystem> +#include <fstream> namespace openpower { @@ -8,6 +12,7 @@ namespace pels { using namespace phosphor::logging; +namespace fs = std::filesystem; namespace additional_data { @@ -36,6 +41,55 @@ void Manager::create(const std::string& message, uint32_t obmcLogID, void Manager::addRawPEL(const std::string& rawPelPath, uint32_t obmcLogID) { + if (fs::exists(rawPelPath)) + { + std::ifstream file(rawPelPath, std::ios::in | std::ios::binary); + + auto data = std::vector<uint8_t>(std::istreambuf_iterator<char>(file), + std::istreambuf_iterator<char>()); + if (file.fail()) + { + log<level::ERR>("Filesystem error reading a raw PEL", + entry("PELFILE=%s", rawPelPath.c_str()), + entry("OBMCLOGID=%d", obmcLogID)); + // TODO, Decide what to do here. Maybe nothing. + return; + } + + file.close(); + + auto pel = std::make_unique<PEL>(data, obmcLogID); + if (pel->valid()) + { + // PELs created by others still need these fields set by us. + pel->assignID(); + pel->setCommitTime(); + + try + { + _repo.add(pel); + } + catch (std::exception& e) + { + // Probably a full or r/o filesystem, not much we can do. + log<level::ERR>("Unable to add PEL to Repository", + entry("PEL_ID=0x%X", pel->id())); + } + } + else + { + log<level::ERR>("Invalid PEL found", + entry("PELFILE=%s", rawPelPath.c_str()), + entry("OBMCLOGID=%d", obmcLogID)); + // TODO, make a whole new OpenBMC event log + PEL + } + } + else + { + log<level::ERR>("Raw PEL file from BMC event log does not exist", + entry("PELFILE=%s", (rawPelPath).c_str()), + entry("OBMCLOGID=%d", obmcLogID)); + } } void Manager::erase(uint32_t obmcLogID) diff --git a/extensions/openpower-pels/manager.hpp b/extensions/openpower-pels/manager.hpp index e57be98..3134f23 100644 --- a/extensions/openpower-pels/manager.hpp +++ b/extensions/openpower-pels/manager.hpp @@ -2,6 +2,8 @@ #include "elog_entry.hpp" #include "log_manager.hpp" +#include "paths.hpp" +#include "repository.hpp" namespace openpower { @@ -28,7 +30,8 @@ class Manager * * @param[in] logManager - internal::Manager object */ - explicit Manager(internal::Manager& logManager) : _logManager(logManager) + explicit Manager(internal::Manager& logManager) : + _logManager(logManager), _repo(getPELRepoPath()) { } @@ -96,6 +99,11 @@ class Manager * @brief Reference to phosphor-logging's Manager class */ internal::Manager& _logManager; + + /** + * @brief The PEL repository object + */ + Repository _repo; }; } // namespace pels diff --git a/extensions/openpower-pels/openpower-pels.mk b/extensions/openpower-pels/openpower-pels.mk index 04106fd..def28a1 100644 --- a/extensions/openpower-pels/openpower-pels.mk +++ b/extensions/openpower-pels/openpower-pels.mk @@ -6,4 +6,5 @@ phosphor_log_manager_SOURCES += \ extensions/openpower-pels/paths.cpp \ extensions/openpower-pels/pel.cpp \ extensions/openpower-pels/private_header.cpp \ + extensions/openpower-pels/repository.cpp \ extensions/openpower-pels/user_header.cpp diff --git a/extensions/openpower-pels/paths.cpp b/extensions/openpower-pels/paths.cpp index dab73c9..27e7373 100644 --- a/extensions/openpower-pels/paths.cpp +++ b/extensions/openpower-pels/paths.cpp @@ -18,6 +18,13 @@ fs::path getPELIDFile() return logIDPath; } +fs::path getPELRepoPath() +{ + std::filesystem::path repoPath{EXTENSION_PERSIST_DIR}; + repoPath /= "pels"; + return repoPath; +} + } // namespace pels } // namespace openpower diff --git a/extensions/openpower-pels/paths.hpp b/extensions/openpower-pels/paths.hpp index 334165c..64566d2 100644 --- a/extensions/openpower-pels/paths.hpp +++ b/extensions/openpower-pels/paths.hpp @@ -11,5 +11,10 @@ namespace pels */ std::filesystem::path getPELIDFile(); +/** + * @brief Returns the path to the PEL repository + */ +std::filesystem::path getPELRepoPath(); + } // namespace pels } // namespace openpower diff --git a/extensions/openpower-pels/repository.cpp b/extensions/openpower-pels/repository.cpp new file mode 100644 index 0000000..c9f9521 --- /dev/null +++ b/extensions/openpower-pels/repository.cpp @@ -0,0 +1,71 @@ +#include "repository.hpp" + +#include <fstream> +#include <phosphor-logging/log.hpp> +#include <xyz/openbmc_project/Common/File/error.hpp> + +namespace openpower +{ +namespace pels +{ + +namespace fs = std::filesystem; +using namespace phosphor::logging; +namespace file_error = sdbusplus::xyz::openbmc_project::Common::File::Error; + +Repository::Repository(const std::filesystem::path& basePath) : + _logPath(basePath / "logs") +{ + if (!fs::exists(_logPath)) + { + fs::create_directories(_logPath); + } +} + +std::string Repository::getPELFilename(uint32_t pelID, const BCDTime& time) +{ + char name[50]; + sprintf(name, "%.2X%.2X%.2X%.2X%.2X%.2X%.2X%.2X_%.8X", time.yearMSB, + time.yearLSB, time.month, time.day, time.hour, time.minutes, + time.seconds, time.hundredths, pelID); + return std::string{name}; +} + +void Repository::add(std::unique_ptr<PEL>& pel) +{ + auto path = _logPath / getPELFilename(pel->id(), pel->commitTime()); + std::ofstream file{path, std::ios::binary}; + + if (!file.good()) + { + // If this fails, the filesystem is probably full so it isn't like + // we could successfully create yet another error log here. + auto e = errno; + log<level::ERR>("Failed creating PEL file", + entry("FILE=%s", path.c_str())); + fs::remove(path); + log<level::ERR>("Unable to open PEL file for writing", + entry("ERRNO=%d", e), entry("PATH=%s", path.c_str())); + throw file_error::Open(); + } + + auto data = pel->data(); + file.write(reinterpret_cast<const char*>(data.data()), data.size()); + + if (file.fail()) + { + // Same note as above about not being able to create an error log + // for this case even if we wanted. + auto e = errno; + log<level::ERR>("Failed writing PEL file", + entry("FILE=%s", path.c_str())); + file.close(); + fs::remove(path); + log<level::ERR>("Unable to write PEL file", entry("ERRNO=%d", e), + entry("PATH=%s", path.c_str())); + throw file_error::Write(); + } +} + +} // namespace pels +} // namespace openpower diff --git a/extensions/openpower-pels/repository.hpp b/extensions/openpower-pels/repository.hpp new file mode 100644 index 0000000..5b15506 --- /dev/null +++ b/extensions/openpower-pels/repository.hpp @@ -0,0 +1,62 @@ +#pragma once +#include "bcd_time.hpp" +#include "pel.hpp" + +#include <algorithm> +#include <filesystem> + +namespace openpower +{ +namespace pels +{ + +/** + * @class Repository + * + * The class handles saving and retrieving PELs on the BMC. + */ +class Repository +{ + public: + Repository() = delete; + ~Repository() = default; + Repository(const Repository&) = default; + Repository& operator=(const Repository&) = default; + Repository(Repository&&) = default; + Repository& operator=(Repository&&) = default; + + /** + * @brief Constructor + * + * @param[in] basePath - the base filesystem path for the repository + */ + Repository(const std::filesystem::path& basePath); + + /** + * @brief Adds a PEL to the repository + * + * Throws File.Error.Open or File.Error.Write exceptions on failure + * + * @param[in] pel - the PEL to add + */ + void add(std::unique_ptr<PEL>& pel); + + /** + * @brief Generates the filename to use for the PEL ID and BCDTime. + * + * @param[in] pelID - the PEL ID + * @param[in] time - the BCD time + * + * @return string - A filename string of <BCD_time>_<pelID> + */ + static std::string getPELFilename(uint32_t pelID, const BCDTime& time); + + private: + /** + * @brief The filesystem path to the PEL logs. + */ + const std::filesystem::path _logPath; +}; + +} // namespace pels +} // namespace openpower diff --git a/test/openpower-pels/Makefile.include b/test/openpower-pels/Makefile.include index 052aa2a..48e1872 100644 --- a/test/openpower-pels/Makefile.include +++ b/test/openpower-pels/Makefile.include @@ -5,7 +5,9 @@ check_PROGRAMS += \ bcd_time_test \ log_id_test \ pel_test \ + pel_manager_test \ private_header_test \ + repository_test \ section_header_test \ stream_test \ user_header_test @@ -80,3 +82,24 @@ pel_test_LDADD = \ $(test_ldadd) \ $(pel_objects) pel_test_LDFLAGS = $(test_ldflags) + +repository_test_SOURCES = \ + %reldir%/repository_test.cpp %reldir%/paths.cpp %reldir%/pel_utils.cpp +repository_test_CPPFLAGS = $(test_cppflags) +repository_test_CXXFLAGS = $(test_cxxflags) +repository_test_LDADD = \ + $(test_ldadd) \ + $(pel_objects) \ + $(top_builddir)/extensions/openpower-pels/repository.o +repository_test_LDFLAGS = $(test_ldflags) + +pel_manager_test_SOURCES = \ + %reldir%/pel_manager_test.cpp %reldir%/paths.cpp %reldir%/pel_utils.cpp +pel_manager_test_CPPFLAGS = $(test_cppflags) +pel_manager_test_CXXFLAGS = $(test_cxxflags) +pel_manager_test_LDADD = \ + $(test_ldadd) \ + $(pel_objects) \ + $(top_builddir)/extensions/openpower-pels/manager.o \ + $(top_builddir)/extensions/openpower-pels/repository.o +pel_manager_test_LDFLAGS = $(test_ldflags)
\ No newline at end of file diff --git a/test/openpower-pels/paths.cpp b/test/openpower-pels/paths.cpp index 464b92c..27f4bce 100644 --- a/test/openpower-pels/paths.cpp +++ b/test/openpower-pels/paths.cpp @@ -22,5 +22,18 @@ std::filesystem::path getPELIDFile() return idFile; } +std::filesystem::path getPELRepoPath() +{ + static std::string repoPath; + + if (repoPath.empty()) + { + char templ[] = "/tmp/repopathtestXXXXXX"; + std::filesystem::path dir = mkdtemp(templ); + repoPath = dir; + } + return repoPath; +} + } // namespace pels } // namespace openpower diff --git a/test/openpower-pels/pel_manager_test.cpp b/test/openpower-pels/pel_manager_test.cpp new file mode 100644 index 0000000..a5b16f5 --- /dev/null +++ b/test/openpower-pels/pel_manager_test.cpp @@ -0,0 +1,66 @@ +#include "extensions/openpower-pels/manager.hpp" +#include "log_manager.hpp" +#include "pel_utils.hpp" + +#include <fstream> +#include <regex> + +#include <gtest/gtest.h> + +using namespace openpower::pels; +namespace fs = std::filesystem; + +class ManagerTest : public CleanPELFiles +{ +}; + +fs::path makeTempDir() +{ + char path[] = "/tmp/tempnameXXXXXX"; + std::filesystem::path dir = mkdtemp(path); + return dir; +} + +// Test that using the RAWPEL=<file> with the Manager::create() call gets +// a PEL saved in the repository. +TEST_F(ManagerTest, TestCreateWithPEL) +{ + auto bus = sdbusplus::bus::new_default(); + phosphor::logging::internal::Manager logManager(bus, "logging_path"); + + openpower::pels::Manager manager{logManager}; + + // Create a PEL, write it to a file, and pass that filename into + // the create function. + auto data = pelDataFactory(TestPelType::pelSimple); + + fs::path pelFilename = makeTempDir() / "rawpel"; + std::ofstream pelFile{pelFilename}; + pelFile.write(reinterpret_cast<const char*>(data->data()), data->size()); + pelFile.close(); + + std::string adItem = "RAWPEL=" + pelFilename.string(); + std::vector<std::string> additionalData{adItem}; + std::vector<std::string> associations; + + manager.create("error message", 42, 0, Entry::Level::Error, additionalData, + associations); + + // We don't know the exact name, but a file should have been added to the + // repo of the form <timestamp>_<ID> + std::regex expr{"\\d+_\\d+"}; + + bool found = false; + for (auto& f : fs::directory_iterator(getPELRepoPath() / "logs")) + { + if (std::regex_search(f.path().string(), expr)) + { + found = true; + break; + } + } + + EXPECT_TRUE(found); + + fs::remove_all(pelFilename.parent_path()); +} diff --git a/test/openpower-pels/pel_utils.cpp b/test/openpower-pels/pel_utils.cpp index 4541694..46ec8ed 100644 --- a/test/openpower-pels/pel_utils.cpp +++ b/test/openpower-pels/pel_utils.cpp @@ -11,6 +11,8 @@ namespace fs = std::filesystem; using namespace openpower::pels; std::filesystem::path CleanLogID::pelIDFile{}; +std::filesystem::path CleanPELFiles::pelIDFile{}; +std::filesystem::path CleanPELFiles::repoPath{}; constexpr uint8_t simplePEL[] = { // private header section header diff --git a/test/openpower-pels/pel_utils.hpp b/test/openpower-pels/pel_utils.hpp index 7475720..262a6e7 100644 --- a/test/openpower-pels/pel_utils.hpp +++ b/test/openpower-pels/pel_utils.hpp @@ -26,6 +26,26 @@ class CleanLogID : public ::testing::Test static std::filesystem::path pelIDFile; }; +class CleanPELFiles : public ::testing::Test +{ + protected: + static void SetUpTestCase() + { + pelIDFile = openpower::pels::getPELIDFile(); + repoPath = openpower::pels::getPELRepoPath(); + } + + static void TearDownTestCase() + { + std::filesystem::remove_all( + std::filesystem::path{pelIDFile}.parent_path()); + std::filesystem::remove_all(repoPath); + } + + static std::filesystem::path pelIDFile; + static std::filesystem::path repoPath; +}; + /** * @brief Tells the factory which PEL to create */ diff --git a/test/openpower-pels/repository_test.cpp b/test/openpower-pels/repository_test.cpp new file mode 100644 index 0000000..97019e8 --- /dev/null +++ b/test/openpower-pels/repository_test.cpp @@ -0,0 +1,73 @@ +#include "extensions/openpower-pels/paths.hpp" +#include "extensions/openpower-pels/repository.hpp" +#include "pel_utils.hpp" + +#include <ext/stdio_filebuf.h> + +#include <filesystem> + +#include <gtest/gtest.h> + +using namespace openpower::pels; +namespace fs = std::filesystem; + +/** + * Clean the Repo after every testcase. + * And because we have PEL object, also clean up + * the log ID. + */ +class RepositoryTest : public CleanLogID +{ + protected: + void SetUp() override + { + repoPath = getPELRepoPath(); + } + + void TearDown() override + { + fs::remove_all(repoPath); + } + + fs::path repoPath; +}; + +TEST_F(RepositoryTest, FilenameTest) +{ + BCDTime date = {0x20, 0x30, 0x11, 0x28, 0x13, 0x6, 0x7, 0x8}; + + EXPECT_EQ(Repository::getPELFilename(0x12345678, date), + "2030112813060708_12345678"); + + EXPECT_EQ(Repository::getPELFilename(0xAABBCCDD, date), + "2030112813060708_AABBCCDD"); + + EXPECT_EQ(Repository::getPELFilename(0x3AFF1, date), + "2030112813060708_0003AFF1"); + + EXPECT_EQ(Repository::getPELFilename(100, date), + "2030112813060708_00000064"); + + EXPECT_EQ(Repository::getPELFilename(0, date), "2030112813060708_00000000"); +} + +TEST_F(RepositoryTest, AddTest) +{ + Repository repo{repoPath}; + auto data = pelDataFactory(TestPelType::pelSimple); + auto pel = std::make_unique<PEL>(*data); + + repo.add(pel); + + // Check that the PEL was stored where it was supposed to be, + // and that it wrote the PEL data. + const auto& ts = pel->privateHeader()->commitTimestamp(); + auto name = Repository::getPELFilename(pel->id(), ts); + + fs::path file = repoPath / "logs" / name; + EXPECT_TRUE(fs::exists(file)); + + auto newData = readPELFile(file); + auto pelData = pel->data(); + EXPECT_EQ(*newData, pelData); +} |