PEL: Skip purging PEL if it has associated hw isolation entry
When PELs reaches to certain limits, either in total size or
total number of them, the code starts removing older pels
based on certain criteria. During this process make sure
no pel is removed that has an associated hardware isolation
entry record or error_log event record.
Signed-off-by: Sumit Kumar <sumit_kumar@in.ibm.com>
Change-Id: Ia7c9af6911bcf2ac8f8ec046d4c7a7a12d9b2bbd
diff --git a/extensions/openpower-pels/data_interface.cpp b/extensions/openpower-pels/data_interface.cpp
index 9f0a218..fa7b087 100644
--- a/extensions/openpower-pels/data_interface.cpp
+++ b/extensions/openpower-pels/data_interface.cpp
@@ -84,11 +84,13 @@
constexpr auto operationalStatus =
"xyz.openbmc_project.State.Decorator.OperationalStatus";
constexpr auto logSetting = "xyz.openbmc_project.Logging.Settings";
-constexpr auto association = "xyz.openbmc_project.Association.Definitions";
+constexpr auto associationDef = "xyz.openbmc_project.Association.Definitions";
constexpr auto dumpEntry = "xyz.openbmc_project.Dump.Entry";
constexpr auto dumpProgress = "xyz.openbmc_project.Common.Progress";
constexpr auto hwIsolationCreate = "org.open_power.HardwareIsolation.Create";
constexpr auto bootRawProgress = "xyz.openbmc_project.State.Boot.Raw";
+constexpr auto hwIsolationEntry = "xyz.openbmc_project.HardwareIsolation.Entry";
+constexpr auto association = "xyz.openbmc_project.Association";
} // namespace interface
using namespace sdbusplus::xyz::openbmc_project::State::Boot::server;
@@ -552,9 +554,9 @@
{
DBusValue getAssociationValue;
- auto service = getService(objectPath, interface::association);
+ auto service = getService(objectPath, interface::associationDef);
- getProperty(service, objectPath, interface::association, "Associations",
+ getProperty(service, objectPath, interface::associationDef, "Associations",
getAssociationValue);
auto association = std::get<AssociationsProperty>(getAssociationValue);
@@ -572,7 +574,7 @@
auto method = _bus.new_method_call(service.c_str(), objectPath.c_str(),
interface::dbusProperty, "Set");
- method.append(interface::association, "Associations",
+ method.append(interface::associationDef, "Associations",
setAssociationValue);
_bus.call(method);
}
@@ -749,5 +751,96 @@
_bus.call(method);
}
+
+std::vector<uint32_t> DataInterface::getLogIDWithHwIsolation() const
+{
+ std::vector<std::string> association = {"xyz.openbmc_project.Association"};
+ std::string hwErrorLog = "/isolated_hw_errorlog";
+ std::string errorLog = "/error_log";
+ DBusPathList paths;
+ std::vector<uint32_t> ids;
+
+ // Get all latest mapper associations
+ paths = getPaths(association);
+ for (auto& path : paths)
+ {
+ // Look for object path with hardware isolation entry if any
+ size_t pos = path.find(hwErrorLog);
+ if (pos != std::string::npos)
+ {
+ // Get the object path
+ std::string ph = path;
+ ph.erase(pos, hwErrorLog.length());
+ auto service = getService(ph, interface::hwIsolationEntry);
+ if (!service.empty())
+ {
+ bool status;
+ DBusValue value;
+
+ // Read the Resolved property from object path
+ getProperty(service, ph, interface::hwIsolationEntry,
+ "Resolved", value);
+
+ status = std::get<bool>(value);
+
+ // If the entry isn't resolved
+ if (!status)
+ {
+ auto service = getService(path, interface::association);
+ if (!service.empty())
+ {
+ DBusValue value;
+
+ // Read Endpoints property
+ getProperty(service, path, interface::association,
+ "endpoints", value);
+
+ auto logPath =
+ std::get<std::vector<std::string>>(value);
+ if (!logPath.empty())
+ {
+ // Get OpenBMC event log Id
+ uint32_t id = stoi(logPath[0].substr(
+ logPath[0].find_last_of('/') + 1));
+ ids.push_back(id);
+ }
+ }
+ }
+ }
+ }
+
+ // Look for object path with error_log entry if any
+ pos = path.find(errorLog);
+ if (pos != std::string::npos)
+ {
+ auto service = getService(path, interface::association);
+ if (!service.empty())
+ {
+ DBusValue value;
+
+ // Read Endpoints property
+ getProperty(service, path, interface::association, "endpoints",
+ value);
+
+ auto logPath = std::get<std::vector<std::string>>(value);
+ if (!logPath.empty())
+ {
+ // Get OpenBMC event log Id
+ uint32_t id = stoi(
+ logPath[0].substr(logPath[0].find_last_of('/') + 1));
+ ids.push_back(id);
+ }
+ }
+ }
+ }
+
+ if (ids.size() > 1)
+ {
+ // remove duplicates to have only unique ids
+ std::sort(ids.begin(), ids.end());
+ ids.erase(std::unique(ids.begin(), ids.end()), ids.end());
+ }
+ return ids;
+}
} // namespace pels
} // namespace openpower
diff --git a/extensions/openpower-pels/data_interface.hpp b/extensions/openpower-pels/data_interface.hpp
index b1f1e92..4095cce 100644
--- a/extensions/openpower-pels/data_interface.hpp
+++ b/extensions/openpower-pels/data_interface.hpp
@@ -367,6 +367,14 @@
createProgressSRC(const uint64_t& priSRC,
const std::vector<uint8_t>& srcStruct) const = 0;
+ /**
+ * @brief Get the list of unresolved OpenBMC event log ids that have an
+ * associated hardware isolation entry.
+ *
+ * @return std::vector<uint32_t> - The list of log ids
+ */
+ virtual std::vector<uint32_t> getLogIDWithHwIsolation() const = 0;
+
protected:
/**
* @brief Sets the host on/off state and runs any
@@ -677,6 +685,14 @@
createProgressSRC(const uint64_t& priSRC,
const std::vector<uint8_t>& srcStruct) const override;
+ /**
+ * @brief Get the list of unresolved OpenBMC event log ids that have an
+ * associated hardware isolation entry.
+ *
+ * @return std::vector<uint32_t> - The list of log ids
+ */
+ std::vector<uint32_t> getLogIDWithHwIsolation() const override;
+
private:
/**
* @brief Reads the BMC firmware version string and puts it into
diff --git a/extensions/openpower-pels/manager.cpp b/extensions/openpower-pels/manager.cpp
index 6bbdf11..419d087 100644
--- a/extensions/openpower-pels/manager.cpp
+++ b/extensions/openpower-pels/manager.cpp
@@ -526,7 +526,9 @@
void Manager::pruneRepo(sdeventplus::source::EventBase& /*source*/)
{
- auto idsToDelete = _repo.prune();
+ auto idsWithHwIsoEntry = _dataIface->getLogIDWithHwIsolation();
+
+ auto idsToDelete = _repo.prune(idsWithHwIsoEntry);
// Remove the OpenBMC event logs for the PELs that were just removed.
std::for_each(idsToDelete.begin(), idsToDelete.end(),
diff --git a/extensions/openpower-pels/repository.cpp b/extensions/openpower-pels/repository.cpp
index 1e9bd6f..b418c39 100644
--- a/extensions/openpower-pels/repository.cpp
+++ b/extensions/openpower-pels/repository.cpp
@@ -582,7 +582,8 @@
return attributes;
}
-std::vector<uint32_t> Repository::prune()
+std::vector<uint32_t>
+ Repository::prune(const std::vector<uint32_t>& idsWithHwIsoEntry)
{
std::vector<uint32_t> obmcLogIDs;
std::string msg = "Pruning PEL repository that takes up " +
@@ -650,16 +651,19 @@
// Check all 4 categories, which will result in at most 90%
// usage (15 + 30 + 15 + 30).
- removePELs(overBMCInfoLimit, isBMCInfo, obmcLogIDs);
- removePELs(overBMCNonInfoLimit, isBMCNonInfo, obmcLogIDs);
- removePELs(overNonBMCInfoLimit, isNonBMCInfo, obmcLogIDs);
- removePELs(overNonBMCNonInfoLimit, isNonBMCNonInfo, obmcLogIDs);
+ removePELs(overBMCInfoLimit, isBMCInfo, idsWithHwIsoEntry, obmcLogIDs);
+ removePELs(overBMCNonInfoLimit, isBMCNonInfo, idsWithHwIsoEntry,
+ obmcLogIDs);
+ removePELs(overNonBMCInfoLimit, isNonBMCInfo, idsWithHwIsoEntry,
+ obmcLogIDs);
+ removePELs(overNonBMCNonInfoLimit, isNonBMCNonInfo, idsWithHwIsoEntry,
+ obmcLogIDs);
// After the above pruning check if there are still too many PELs,
// which can happen depending on PEL sizes.
if (_pelAttributes.size() > _maxNumPELs)
{
- removePELs(tooManyPELsLimit, isAnyPEL, obmcLogIDs);
+ removePELs(tooManyPELsLimit, isAnyPEL, idsWithHwIsoEntry, obmcLogIDs);
}
if (!obmcLogIDs.empty())
@@ -674,6 +678,7 @@
void Repository::removePELs(IsOverLimitFunc& isOverLimit,
IsPELTypeFunc& isPELType,
+ const std::vector<uint32_t>& idsWithHwIsoEntry,
std::vector<uint32_t>& removedBMCLogIDs)
{
if (!isOverLimit())
@@ -712,6 +717,15 @@
if (isPELType(pel.second) && stateCheck(pel.second))
{
auto removedID = pel.first.obmcID.id;
+
+ auto idFound = std::find(idsWithHwIsoEntry.begin(),
+ idsWithHwIsoEntry.end(), removedID);
+ if (idFound != idsWithHwIsoEntry.end())
+ {
+ ++it;
+ continue;
+ }
+
remove(pel.first);
removedBMCLogIDs.push_back(removedID);
diff --git a/extensions/openpower-pels/repository.hpp b/extensions/openpower-pels/repository.hpp
index b2fc797..2e8d2d6 100644
--- a/extensions/openpower-pels/repository.hpp
+++ b/extensions/openpower-pels/repository.hpp
@@ -402,10 +402,12 @@
* Pass 3: only delete PHYP sent PELs
* Pass 4: delete all PELs
*
+ * @param[in] ids - The OpenBMC event log Ids with hardware isolation entry.
+ *
* @return std::vector<uint32_t> - The OpenBMC event log IDs of
* the PELs that were deleted.
*/
- std::vector<uint32_t> prune();
+ std::vector<uint32_t> prune(const std::vector<uint32_t>& idsWithHwIsoEntry);
/**
* @brief Returns the path to the directory where the PEL
@@ -558,11 +560,14 @@
* removed.
* @param[in] isPELType - The bool(const PELAttributes&) function
* used to select the PELs to operate on.
+ * @param[in] ids - The OpenBMC event log Ids with hardware isolation
+ * entry.
*
* @param[out] removedBMCLogIDs - The OpenBMC event log IDs of the
* removed PELs.
*/
void removePELs(IsOverLimitFunc& isOverLimit, IsPELTypeFunc& isPELType,
+ const std::vector<uint32_t>& idsWithHwIsoEntry,
std::vector<uint32_t>& removedBMCLogIDs);
/**
* @brief The filesystem path to the PEL logs.
diff --git a/test/openpower-pels/mocks.hpp b/test/openpower-pels/mocks.hpp
index 63786c4..4ff0f4f 100644
--- a/test/openpower-pels/mocks.hpp
+++ b/test/openpower-pels/mocks.hpp
@@ -57,6 +57,8 @@
MOCK_METHOD(void, createProgressSRC,
(const uint64_t&, const std::vector<uint8_t>&),
(const override));
+ MOCK_METHOD(std::vector<uint32_t>, getLogIDWithHwIsolation, (),
+ (const override));
void changeHostState(bool newState)
{
diff --git a/test/openpower-pels/repository_test.cpp b/test/openpower-pels/repository_test.cpp
index 7aa0d0d..73a24d8 100644
--- a/test/openpower-pels/repository_test.cpp
+++ b/test/openpower-pels/repository_test.cpp
@@ -624,6 +624,7 @@
// Prune PELs, when no HMC/OS/PHYP acks
TEST_F(RepositoryTest, TestPruneNoAcks)
{
+ std::vector<uint32_t> id;
Repository repo{repoPath, 4096 * 20, 100};
// Add 10 4096B (on disk) PELs of BMC nonInfo, Info and nonBMC info,
@@ -663,7 +664,7 @@
}
// Prune down to 15%/30%/15%/30% = 90% total
- auto IDs = repo.prune();
+ auto IDs = repo.prune(id);
// Check the final sizes
EXPECT_EQ(sizes.total, 4096 * 18); // 90% of 20 PELs
@@ -689,6 +690,7 @@
// pruning still works properly
TEST_F(RepositoryTest, TestPruneInfoOnly)
{
+ std::vector<uint32_t> id;
Repository repo{repoPath, 4096 * 22, 100};
// Fill 4096*23 bytes on disk of BMC info PELs
@@ -712,7 +714,7 @@
EXPECT_TRUE(repo.getPELAttributes(id));
}
- auto IDs = repo.prune();
+ auto IDs = repo.prune(id);
// Check the final sizes
EXPECT_EQ(sizes.total, 4096 * 3);
@@ -736,6 +738,7 @@
// pruning order.
TEST_F(RepositoryTest, TestPruneWithAcks)
{
+ std::vector<uint32_t> id;
Repository repo{repoPath, 4096 * 20, 100};
// Fill 30% worth of BMC non-info non-acked PELs
@@ -770,7 +773,7 @@
repo.setPELHostTransState(pel->id(), TransmissionState::sent);
}
- auto IDs = repo.prune();
+ auto IDs = repo.prune(id);
EXPECT_EQ(repo.getSizeStats().total, 4096 * 6);
// The newest PEL should be the one deleted
@@ -782,6 +785,7 @@
// Test that the total number of PELs limit is enforced.
TEST_F(RepositoryTest, TestPruneTooManyPELs)
{
+ std::vector<uint32_t> id;
Repository repo{repoPath, 4096 * 100, 10};
// Add 10, which is the limit and is still OK
@@ -792,7 +796,7 @@
repo.add(pel);
}
- auto IDs = repo.prune();
+ auto IDs = repo.prune(id);
// Nothing pruned yet
EXPECT_TRUE(IDs.empty());
@@ -806,7 +810,7 @@
// Now that's it's over the limit of 10, it will bring it down
// to 80%, which is 8 after it removes 3.
- IDs = repo.prune();
+ IDs = repo.prune(id);
EXPECT_EQ(repo.getSizeStats().total, 4096 * 8);
ASSERT_EQ(IDs.size(), 3);
@@ -1011,3 +1015,43 @@
logID = repo.getLogID(idWithObmcLogId);
ASSERT_TRUE(!logID.has_value());
}
+
+// Test that OpenBMC log Id with hardware isolation entry is not removed.
+TEST_F(RepositoryTest, TestPruneWithIdHwIsoEntry)
+{
+ std::vector<uint32_t> id{502};
+ Repository repo{repoPath, 4096 * 100, 10};
+
+ // Add 10, which is the limit and is still OK
+ for (uint32_t i = 1; i <= 10; i++)
+ {
+ auto data = pelFactory(i, 'O', 0x20, 0x8800, 500);
+ auto pel = std::make_unique<PEL>(data);
+ repo.add(pel);
+ }
+
+ auto IDs = repo.prune(id);
+
+ // Nothing pruned yet
+ EXPECT_TRUE(IDs.empty());
+
+ // Add 1 more PEL which will be too many.
+ {
+ auto data = pelFactory(11, 'O', 0x20, 0x8800, 500);
+ auto pel = std::make_unique<PEL>(data);
+ repo.add(pel);
+ }
+
+ // Now that's it's over the limit of 10, it will bring it down
+ // to 80%, which is 8 after it removes 3.
+ IDs = repo.prune(id);
+ EXPECT_EQ(repo.getSizeStats().total, 4096 * 8);
+ ASSERT_EQ(IDs.size(), 3);
+
+ // Check that it deleted the oldest ones.
+ // And the Id with hw isolation entry is NOT removed.
+ // The OpenBMC log ID is the PEL ID + 500.
+ EXPECT_EQ(IDs[0], 500 + 1);
+ EXPECT_EQ(IDs[1], 500 + 3);
+ EXPECT_EQ(IDs[2], 500 + 4);
+}