PEL: Guard against hostboot sending down duplicate PEL Ids
This commit is to guard against hostboot sending down PEL Id
that we already have in our repository. This caused PEL to
get orphaned in the filesystem without a corresponding valid
openbmc event log Id.
The action is to move such PELs to archive folder.
Signed-off-by: Sumit Kumar <sumit_kumar@in.ibm.com>
Change-Id: I46bb865b4b87ec0b59f362e3f79a1b5a6710a45c
diff --git a/extensions/openpower-pels/manager.cpp b/extensions/openpower-pels/manager.cpp
index 3387970..db2e2d6 100644
--- a/extensions/openpower-pels/manager.cpp
+++ b/extensions/openpower-pels/manager.cpp
@@ -127,7 +127,6 @@
void Manager::addPEL(std::vector<uint8_t>& pelData, uint32_t obmcLogID)
{
-
auto pel = std::make_unique<openpower::pels::PEL>(pelData, obmcLogID);
if (pel->valid())
{
@@ -137,6 +136,22 @@
{
pel->assignID();
}
+ else
+ {
+ const Repository::LogID id{Repository::LogID::Pel(pel->id())};
+ auto result = _repo.hasPEL(id);
+ if (result)
+ {
+ log<level::WARNING>(
+ fmt::format("Duplicate HostBoot PEL Id {:#X} found; "
+ "moving it to archive folder",
+ pel->id())
+ .c_str());
+
+ _repo.archivePEL(*pel);
+ return;
+ }
+ }
// PELs created by others still need this field set by us.
pel->setCommitTime();
diff --git a/extensions/openpower-pels/repository.cpp b/extensions/openpower-pels/repository.cpp
index c49d92f..9b650ce 100644
--- a/extensions/openpower-pels/repository.cpp
+++ b/extensions/openpower-pels/repository.cpp
@@ -734,5 +734,17 @@
}
}
+void Repository::archivePEL(const PEL& pel)
+{
+ if (pel.valid())
+ {
+ auto path = _archivePath / getPELFilename(pel.id(), pel.commitTime());
+
+ write(pel, path);
+
+ _archiveSize += getFileDiskSize(path);
+ }
+}
+
} // namespace pels
} // namespace openpower
diff --git a/extensions/openpower-pels/repository.hpp b/extensions/openpower-pels/repository.hpp
index a57b901..b8ef126 100644
--- a/extensions/openpower-pels/repository.hpp
+++ b/extensions/openpower-pels/repository.hpp
@@ -445,6 +445,13 @@
return std::nullopt;
}
+ /**
+ * @brief Save the PEL to archive folder
+ *
+ * @param[in] pel - The PEL data
+ */
+ void archivePEL(const PEL& pel);
+
private:
using PELUpdateFunc = std::function<void(PEL&)>;
diff --git a/test/openpower-pels/pel_manager_test.cpp b/test/openpower-pels/pel_manager_test.cpp
index 7bf350f..c1cf5ec 100644
--- a/test/openpower-pels/pel_manager_test.cpp
+++ b/test/openpower-pels/pel_manager_test.cpp
@@ -973,3 +973,52 @@
associations);
}
}
+
+// Test for duplicate PELs moved to archive folder
+TEST_F(ManagerTest, TestDuplicatePEL)
+{
+ sdeventplus::Event e{sdEvent};
+ size_t count = 0;
+
+ std::unique_ptr<DataInterfaceBase> dataIface =
+ std::make_unique<MockDataInterface>();
+
+ openpower::pels::Manager manager{
+ logManager, std::move(dataIface),
+ std::bind(std::mem_fn(&TestLogger::log), &logger, std::placeholders::_1,
+ std::placeholders::_2, std::placeholders::_3)};
+
+ for (int i = 0; i < 2; i++)
+ {
+ // This hostboot PEL has a single hardware callout in it.
+ auto data = pelFactory(1, 'B', 0x20, 0xA400, 500);
+
+ 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,
+ phosphor::logging::Entry::Level::Error, additionalData,
+ associations);
+
+ e.run(std::chrono::milliseconds(1));
+ }
+
+ for (auto& f :
+ fs::directory_iterator(getPELRepoPath() / "logs" / "archive"))
+ {
+ if (fs::is_regular_file(f.path()))
+ {
+ count++;
+ }
+ }
+
+ // Get count of PELs in the repository & in archive directtory
+ EXPECT_EQ(countPELsInRepo(), 1);
+ EXPECT_EQ(count, 1);
+}