PEL: Prevent deletion if it's associated with HWIsolation
- This ensures that PELs linked to HWIsolation records are protected
from accidental deletion.
- Shows error message of "Call failed: The service is temporarily
unavailable.", when attempting to delete such a PEL individually.
- If trying to Delete all, will skip such PELs without showing any
message.
Tested:
Sample output:
```bash
$ busctl call xyz.openbmc_project.Logging /xyz/openbmc_project/
logging xyz.openbmc_project.Collection.DeleteAll DeleteAll
$ busctl call xyz.openbmc_project.Logging /xyz/openbmc_project/
logging/entry/2 xyz.openbmc_project.Object.Delete Delete
Call failed: The service is temporarily unavailable.
```
Change-Id: I2d28de91bbb0fbc2a991e3d5e5631814d41fe044
Signed-off-by: Harsh Agarwal <Harsh.Agarwal@ibm.com>
diff --git a/test/openpower-pels/mocks.hpp b/test/openpower-pels/mocks.hpp
index 3b74871..ae8d385 100644
--- a/test/openpower-pels/mocks.hpp
+++ b/test/openpower-pels/mocks.hpp
@@ -68,6 +68,10 @@
MOCK_METHOD(std::vector<uint8_t>, getRawProgressSRC, (), (const override));
MOCK_METHOD(std::optional<std::vector<uint8_t>>, getDIProperty,
(const std::string&), (const override));
+ MOCK_METHOD(DBusPathList, getAssociatedPaths,
+ (const DBusPath&, const DBusPath&, int32_t,
+ const DBusInterfaceList&),
+ (const override));
void changeHostState(bool newState)
{
diff --git a/test/openpower-pels/pel_manager_test.cpp b/test/openpower-pels/pel_manager_test.cpp
index 3689bdf..e45fd55 100644
--- a/test/openpower-pels/pel_manager_test.cpp
+++ b/test/openpower-pels/pel_manager_test.cpp
@@ -31,6 +31,7 @@
using ::testing::NiceMock;
using ::testing::Return;
+using json = nlohmann::json;
class TestLogger
{
@@ -1235,3 +1236,242 @@
mockIface->fruPresent("U1234-A4");
checkDeconfigured(true);
}
+
+int createHWIsolatedCalloutFile()
+{
+ json jsonCalloutDataList(nlohmann::json::value_t::array);
+ json jsonDimmCallout;
+
+ jsonDimmCallout["LocationCode"] = "Ufcs-DIMM0";
+ jsonDimmCallout["EntityPath"] = {35, 1, 0, 2, 0, 3, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+ jsonDimmCallout["GuardType"] = "GARD_Predictive";
+ jsonDimmCallout["Deconfigured"] = false;
+ jsonDimmCallout["Guarded"] = true;
+ jsonDimmCallout["Priority"] = "M";
+ jsonCalloutDataList.emplace_back(std::move(jsonDimmCallout));
+
+ std::string calloutData(jsonCalloutDataList.dump());
+ std::string calloutFile("/tmp/phalPELCalloutsJson.XXXXXX");
+ int fileFD = -1;
+
+ fileFD = mkostemp(calloutFile.data(), O_RDWR);
+ if (fileFD == -1)
+ {
+ perror("Failed to create PELCallouts file");
+ return -1;
+ }
+
+ ssize_t rc = write(fileFD, calloutData.c_str(), calloutData.size());
+ if (rc == -1)
+ {
+ perror("Failed to write PELCallouts file");
+ close(fileFD);
+ return -1;
+ }
+
+ // Ensure we seek to the beginning of the file
+ rc = lseek(fileFD, 0, SEEK_SET);
+ if (rc == -1)
+ {
+ perror("Failed to set SEEK_SET for PELCallouts file");
+ close(fileFD);
+ return -1;
+ }
+ return fileFD;
+}
+
+void appendFFDCEntry(int fd, uint8_t subTypeJson, uint8_t version,
+ phosphor::logging::FFDCEntries& ffdcEntries)
+{
+ phosphor::logging::FFDCEntry ffdcEntry =
+ std::make_tuple(sdbusplus::xyz::openbmc_project::Logging::server::
+ Create::FFDCFormat::JSON,
+ subTypeJson, version, fd);
+ ffdcEntries.push_back(ffdcEntry);
+}
+
+TEST_F(ManagerTest, TestPELDeleteWithoutHWIsolation)
+{
+ const auto registry = R"(
+ {
+ "PELs":
+ [{
+ "Name": "xyz.openbmc_project.Error.Test",
+ "SRC":
+ {
+ "ReasonCode": "0x2030"
+ },
+ "Documentation": {
+ "Description": "Test Error",
+ "Message": "Test Error"
+ }
+ }]
+ }
+ )";
+
+ auto path = getPELReadOnlyDataPath();
+ fs::create_directories(path);
+ path /= "message_registry.json";
+
+ std::ofstream registryFile{path};
+ registryFile << registry;
+ registryFile.close();
+
+ std::unique_ptr<DataInterfaceBase> dataIface =
+ std::make_unique<MockDataInterface>();
+
+ MockDataInterface* mockIface =
+ reinterpret_cast<MockDataInterface*>(dataIface.get());
+
+ EXPECT_CALL(*mockIface, getInventoryFromLocCode("Ufcs-DIMM0", 0, false))
+ .WillOnce(Return(std::vector<std::string>{
+ "/xyz/openbmc_project/inventory/system/chassis/motherboard/dimm0"}));
+
+ // Mock the scenario where the hardware isolation guard is flagged
+ // but is not associated, resulting in an empty list being returned.
+ EXPECT_CALL(
+ *mockIface,
+ getAssociatedPaths(
+ ::testing::StrEq(
+ "/xyz/openbmc_project/logging/entry/42/isolated_hw_entry"),
+ ::testing::StrEq("/"), 0,
+ ::testing::ElementsAre(
+ "xyz.openbmc_project.HardwareIsolation.Entry")))
+ .WillRepeatedly(Return(std::vector<std::string>{}));
+
+ std::unique_ptr<JournalBase> journal = std::make_unique<MockJournal>();
+ 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),
+ std::move(journal)};
+ std::vector<std::string> additionalData;
+ std::vector<std::string> associations;
+
+ // Check when there's no PEL with given id.
+ {
+ EXPECT_FALSE(manager.isDeleteProhibited(42));
+ }
+ // creating without ffdcEntries
+ manager.create("xyz.openbmc_project.Error.Test", 42, 0,
+ phosphor::logging::Entry::Level::Error, additionalData,
+ associations);
+ auto pelFile = findAnyPELInRepo();
+ auto data = readPELFile(*pelFile);
+ PEL pel_unguarded(*data);
+ {
+ // Verify that the guard flag is false.
+ EXPECT_FALSE(pel_unguarded.getGuardFlag());
+ // Check that `isDeleteProhibited` returns false when the guard flag is
+ // false.
+ EXPECT_FALSE(manager.isDeleteProhibited(42));
+ }
+ manager.erase(42);
+ EXPECT_FALSE(findAnyPELInRepo());
+
+ int fd = createHWIsolatedCalloutFile();
+ ASSERT_NE(fd, -1);
+ uint8_t subTypeJson = 0xCA;
+ uint8_t version = 0x01;
+ phosphor::logging::FFDCEntries ffdcEntries;
+ appendFFDCEntry(fd, subTypeJson, version, ffdcEntries);
+ manager.create("xyz.openbmc_project.Error.Test", 42, 0,
+ phosphor::logging::Entry::Level::Error, additionalData,
+ associations, ffdcEntries);
+ close(fd);
+
+ auto pelPathInRepo = findAnyPELInRepo();
+ auto unguardedData = readPELFile(*pelPathInRepo);
+ PEL pel(*unguardedData);
+ {
+ // Verify guard flag set to true
+ EXPECT_TRUE(pel.getGuardFlag());
+ // Check even if guard flag is true, if dbus call returns empty
+ // array list then `isDeleteProhibited` returns false
+ EXPECT_FALSE(manager.isDeleteProhibited(42));
+ }
+ manager.erase(42);
+}
+
+TEST_F(ManagerTest, TestPELDeleteWithHWIsolation)
+{
+ const auto registry = R"(
+ {
+ "PELs":
+ [{
+ "Name": "xyz.openbmc_project.Error.Test",
+ "Severity": "critical_system_term",
+ "SRC":
+ {
+ "ReasonCode": "0x2030"
+ },
+ "Documentation": {
+ "Description": "Test Error",
+ "Message": "Test Error"
+ }
+ }]
+ }
+ )";
+
+ auto path = getPELReadOnlyDataPath();
+ fs::create_directories(path);
+ path /= "message_registry.json";
+
+ std::ofstream registryFile{path};
+ registryFile << registry;
+ registryFile.close();
+
+ std::unique_ptr<DataInterfaceBase> dataIface =
+ std::make_unique<MockDataInterface>();
+
+ MockDataInterface* mockIface =
+ reinterpret_cast<MockDataInterface*>(dataIface.get());
+
+ EXPECT_CALL(*mockIface, getInventoryFromLocCode("Ufcs-DIMM0", 0, false))
+ .WillOnce(Return(std::vector<std::string>{
+ "/xyz/openbmc_project/inventory/system/chassis/motherboard/dimm0"}));
+
+ EXPECT_CALL(
+ *mockIface,
+ getAssociatedPaths(
+ ::testing::StrEq(
+ "/xyz/openbmc_project/logging/entry/42/isolated_hw_entry"),
+ ::testing::StrEq("/"), 0,
+ ::testing::ElementsAre(
+ "xyz.openbmc_project.HardwareIsolation.Entry")))
+ .WillRepeatedly(Return(std::vector<std::string>{
+ "/xyz/openbmc_project/hardware_isolation/entry/1"}));
+
+ std::unique_ptr<JournalBase> journal = std::make_unique<MockJournal>();
+ 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),
+ std::move(journal)};
+ std::vector<std::string> additionalData;
+ std::vector<std::string> associations;
+
+ int fd = createHWIsolatedCalloutFile();
+ ASSERT_NE(fd, -1);
+ uint8_t subTypeJson = 0xCA;
+ uint8_t version = 0x01;
+ phosphor::logging::FFDCEntries ffdcEntries;
+ appendFFDCEntry(fd, subTypeJson, version, ffdcEntries);
+ manager.create("xyz.openbmc_project.Error.Test", 42, 0,
+ phosphor::logging::Entry::Level::Error, additionalData,
+ associations, ffdcEntries);
+ close(fd);
+
+ auto pelFile = findAnyPELInRepo();
+ EXPECT_TRUE(pelFile);
+ auto data = readPELFile(*pelFile);
+ PEL pel(*data);
+ EXPECT_TRUE(pel.valid());
+ // Test case where the guard flag is set to true and the hardware isolation
+ // guard is associated, which should result in `isDeleteProhibited`
+ // returning true as expected.
+ EXPECT_TRUE(pel.getGuardFlag());
+ EXPECT_TRUE(manager.isDeleteProhibited(42));
+ manager.erase(42);
+}