PEL: Still create callout without full loc code
When PELs are created early during a power on reset, the service that
expands location codes may not be online yet. Previously, callouts just
wouldn't be created if a location code couldn't be expanded.
Fix that so that a callout is still created with the unexpanded location
code, as that is still better than nothing. In those cases, the
CC/PN/FN fields would also be empty since those are obtained by finding
the FRU inventory path from the location code.
Also create a similar callout in the case where the call to get the
inventory path from a location code fails.
This applies for callouts created via:
- The PEL message registry
- The device path lookup tables
- JSON FFDC files
Tested:
New unit tests that cover these paths pass.
On a system, after stopping vpd-manager.service, will get callouts like:
```
"Callout Section": {
"Callout Count": "4",
"Callouts": [{
"FRU Type": "Normal Hardware FRU",
"Priority": "Mandatory, replace all with this type as a unit",
"Location Code": "P0-C5",
"Part Number": "",
"CCIN": "",
"Serial Number": ""
}, {
"FRU Type": "Normal Hardware FRU",
"Priority": "Mandatory, replace all with this type as a unit",
"Location Code": "E0",
"Part Number": "",
"CCIN": "",
"Serial Number": ""
}, {
"FRU Type": "Normal Hardware FRU",
"Priority": "Lowest priority replacement",
"Location Code": "P0",
"Part Number": "",
"CCIN": "",
"Serial Number": ""
}, {
"FRU Type": "Normal Hardware FRU",
"Priority": "Lowest priority replacement",
"Location Code": "P0-T4",
"Part Number": "",
"CCIN": "",
"Serial Number": ""
}]
}
```
Change-Id: Ia7e94f9d4848f78ba0b00023438c4db4135fed75
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
diff --git a/extensions/openpower-pels/src.cpp b/extensions/openpower-pels/src.cpp
index 7b7e59f..cfa7155 100644
--- a/extensions/openpower-pels/src.cpp
+++ b/extensions/openpower-pels/src.cpp
@@ -867,6 +867,17 @@
}
}
+void SRC::addLocationCodeOnlyCallout(const std::string& locationCode,
+ const CalloutPriority priority)
+{
+ std::string empty;
+ std::vector<src::MRU::MRUCallout> mrus;
+ auto callout = std::make_unique<src::Callout>(priority, locationCode, empty,
+ empty, empty, mrus);
+ createCalloutsObject();
+ _callouts->addCallout(std::move(callout));
+}
+
void SRC::addInventoryCallout(const std::string& inventoryPath,
const std::optional<CalloutPriority>& priority,
const std::optional<std::string>& locationCode,
@@ -1006,6 +1017,7 @@
{
std::unique_ptr<src::Callout> callout;
auto locCode = regCallout.locCode;
+ bool locExpanded = true;
if (!locCode.empty())
{
@@ -1018,7 +1030,7 @@
auto msg = "Unable to expand location code " + locCode + ": " +
e.what();
addDebugData(msg);
- return;
+ locExpanded = false;
}
}
@@ -1045,6 +1057,7 @@
else if (!regCallout.symbolicFRUTrusted.empty())
{
// Symbolic FRU with trusted location code callout
+ bool trusted = false;
// Use the location code from the inventory path if there is one.
if (trustedSymbolicFRUInvPath)
@@ -1052,6 +1065,7 @@
try
{
locCode = dataIface.getLocationCode(*trustedSymbolicFRUInvPath);
+ trusted = true;
}
catch (const std::exception& e)
{
@@ -1062,14 +1076,29 @@
}
}
+ // Can only trust the location code if it isn't empty and is expanded.
+ if (!locCode.empty() && locExpanded)
+ {
+ trusted = true;
+ }
+
// The registry wants it to be trusted, but that requires a valid
// location code for it to actually be.
callout = std::make_unique<src::Callout>(
- priority, regCallout.symbolicFRUTrusted, locCode, !locCode.empty());
+ priority, regCallout.symbolicFRUTrusted, locCode, trusted);
}
else
{
// A hardware callout
+
+ // If couldn't expand the location code, don't bother
+ // looking up the inventory path.
+ if (!locExpanded && !locCode.empty())
+ {
+ addLocationCodeOnlyCallout(locCode, priority);
+ return;
+ }
+
std::vector<std::string> inventoryPaths;
try
@@ -1084,6 +1113,11 @@
"Unable to get inventory path from location code: " + locCode +
": " + e.what();
addDebugData(msg);
+ if (!locCode.empty())
+ {
+ // Still add a callout with just the location code.
+ addLocationCodeOnlyCallout(locCode, priority);
+ }
return;
}
@@ -1196,6 +1230,10 @@
auto msg = std::format("Unable to expand location code {}: {}",
callout.locationCode, e.what());
addDebugData(msg);
+
+ // Add the callout with just the unexpanded location code.
+ addLocationCodeOnlyCallout(callout.locationCode, priority);
+ continue;
}
try
@@ -1214,6 +1252,9 @@
"Unable to get inventory path from location code: " +
callout.locationCode + ": " + e.what();
addDebugData(msg);
+ // Add the callout with just the location code.
+ addLocationCodeOnlyCallout(callout.locationCode, priority);
+ continue;
}
// Until the code is there to convert these MRU value strings to
@@ -1352,10 +1393,11 @@
}
catch (const std::exception& e)
{
- throw std::runtime_error{
- std::format("Unable to get inventory path from "
- "location code: {}: {}",
- unexpandedLocCode, e.what())};
+ addDebugData(std::format("Unable to get inventory path from "
+ "location code: {}: {}",
+ unexpandedLocCode, e.what()));
+ addLocationCodeOnlyCallout(locCode, priority);
+ return;
}
}
diff --git a/extensions/openpower-pels/src.hpp b/extensions/openpower-pels/src.hpp
index ff302e0..fa3a952 100644
--- a/extensions/openpower-pels/src.hpp
+++ b/extensions/openpower-pels/src.hpp
@@ -566,6 +566,18 @@
const DataInterfaceBase& dataIface);
/**
+ * @brief Adds a FRU callout with only the location code
+ *
+ * Used when it's known not all of the data is available,
+ * including possibly the expanded location code.
+ *
+ * @param[in] locationCode - The location code
+ * @param[in] priority - The callout priority
+ */
+ void addLocationCodeOnlyCallout(const std::string& locationCode,
+ const CalloutPriority priority);
+
+ /**
* @brief Extracts a CalloutPriority value from the json
* using the 'Priority' key.
*
diff --git a/test/openpower-pels/src_test.cpp b/test/openpower-pels/src_test.cpp
index 2c49a7b..44c0f61 100644
--- a/test/openpower-pels/src_test.cpp
+++ b/test/openpower-pels/src_test.cpp
@@ -775,6 +775,118 @@
}
}
+TEST_F(SRCTest, RegistryCalloutCantGetLocTest)
+{
+ message::Entry entry;
+ entry.src.type = 0xBD;
+ entry.src.reasonCode = 0xABCD;
+ entry.src.deconfigFlag = true;
+ entry.src.checkstopFlag = true;
+ entry.subsystem = 0x42;
+
+ entry.callouts = R"(
+ [{
+ "CalloutList":
+ [
+ {
+ "Priority": "high",
+ "LocCode": "P0-C8"
+ },
+ {
+ "Priority": "medium",
+ "LocCode": "P0-C9"
+ }
+ ]
+ }])"_json;
+
+ {
+ // The calls to expand the location codes will fail, but it should
+ // still create the callouts with the unexpanded values and no HW
+ // fields.
+ AdditionalData ad;
+ NiceMock<MockDataInterface> dataIface;
+ std::vector<std::string> names{"systemC"};
+
+ EXPECT_CALL(dataIface, getSystemNames).WillOnce(Return(names));
+
+ EXPECT_CALL(dataIface, expandLocationCode("P0-C8", 0))
+ .WillRepeatedly(Throw(std::runtime_error("Fail")));
+
+ EXPECT_CALL(dataIface, expandLocationCode("P0-C9", 0))
+ .WillRepeatedly(Throw(std::runtime_error("Fail")));
+
+ EXPECT_CALL(dataIface, getInventoryFromLocCode(_, _, _)).Times(0);
+
+ EXPECT_CALL(dataIface, getHWCalloutFields(_, _, _, _)).Times(0);
+
+ SRC src{entry, ad, dataIface};
+
+ auto& callouts = src.callouts()->callouts();
+ ASSERT_EQ(callouts.size(), 2);
+
+ // Only unexpanded location codes
+ EXPECT_EQ(callouts[0]->locationCode(), "P0-C8");
+ EXPECT_EQ(callouts[0]->priority(), 'H');
+
+ auto& fru1 = callouts[0]->fruIdentity();
+ EXPECT_EQ(fru1->getPN().value(), "");
+ EXPECT_EQ(fru1->getCCIN().value(), "");
+ EXPECT_EQ(fru1->getSN().value(), "");
+
+ EXPECT_EQ(callouts[1]->locationCode(), "P0-C9");
+ EXPECT_EQ(callouts[1]->priority(), 'M');
+
+ auto& fru2 = callouts[1]->fruIdentity();
+ EXPECT_EQ(fru2->getPN().value(), "");
+ EXPECT_EQ(fru2->getCCIN().value(), "");
+ EXPECT_EQ(fru2->getSN().value(), "");
+ }
+}
+
+TEST_F(SRCTest, TrustedSymbolicFRUCantGetLocTest)
+{
+ message::Entry entry;
+ entry.src.type = 0xBD;
+ entry.src.reasonCode = 0xABCD;
+ entry.subsystem = 0x42;
+
+ entry.callouts = R"(
+ [{
+ "CalloutList":
+ [
+ {
+ "Priority": "medium",
+ "LocCode": "P0-C8",
+ "SymbolicFRUTrusted": "pwrsply"
+ }
+ ]
+ }])"_json;
+
+ std::map<std::string, std::string> adData;
+ AdditionalData ad{adData};
+ NiceMock<MockDataInterface> dataIface;
+ std::vector<std::string> names{"systemA"};
+
+ EXPECT_CALL(dataIface, getSystemNames).WillOnce(Return(names));
+
+ // The call to expand the location code will fail, but it should
+ // still create the callout with the unexpanded value and the
+ // symbolic FRU can't be trusted.
+ EXPECT_CALL(dataIface, expandLocationCode("P0-C8", 0))
+ .WillRepeatedly(Throw(std::runtime_error("Fail")));
+
+ SRC src{entry, ad, dataIface};
+
+ auto& callouts = src.callouts()->callouts();
+ ASSERT_EQ(callouts.size(), 1);
+
+ EXPECT_EQ(callouts[0]->locationCode(), "P0-C8");
+ EXPECT_EQ(callouts[0]->priority(), 'M');
+ auto& fru = callouts[0]->fruIdentity();
+ EXPECT_EQ(fru->getPN().value(), "PWRSPLY");
+ EXPECT_EQ(fru->failingComponentType(), src::FRUIdentity::symbolicFRU);
+}
+
// Test looking up device path fails in the callout jSON.
TEST_F(SRCTest, DevicePathCalloutTest)
{
@@ -975,6 +1087,96 @@
"Problem looking up I2C callouts on 22 153: "
"[json.exception.out_of_range.403] key '22' not found");
}
+}
+
+TEST_F(SRCTest, DevicePathCantGetLocTest)
+{
+ message::Entry entry;
+ entry.src.type = 0xBD;
+ entry.src.reasonCode = 0xABCD;
+ entry.subsystem = 0x42;
+
+ const auto calloutJSON = R"(
+ {
+ "I2C":
+ {
+ "14":
+ {
+ "114":
+ {
+ "Callouts":[
+ {
+ "Name": "/chassis/motherboard/cpu0",
+ "LocationCode": "P1-C40",
+ "Priority": "H"
+ },
+ {
+ "Name": "/chassis/motherboard",
+ "LocationCode": "P1",
+ "Priority": "M"
+ }
+ ],
+ "Dest": "proc 0 target"
+ }
+ }
+ }
+ })";
+
+ auto dataPath = getPELReadOnlyDataPath();
+ std::ofstream file{dataPath / "systemA_dev_callouts.json"};
+ file << calloutJSON;
+ file.close();
+
+ NiceMock<MockDataInterface> dataIface;
+ std::vector<std::string> names{"systemA"};
+
+ EXPECT_CALL(dataIface, getSystemNames).WillRepeatedly(Return(names));
+
+ // The calls to expand the location codes will fail, so still create
+ // the callouts with the unexpanded values and no HW fields
+
+ EXPECT_CALL(dataIface, expandLocationCode("P1-C40", 0))
+ .WillRepeatedly(Throw(std::runtime_error("Fail")));
+
+ EXPECT_CALL(dataIface, expandLocationCode("P1", 0))
+ .WillRepeatedly(Throw(std::runtime_error("Fail")));
+
+ EXPECT_CALL(dataIface, getInventoryFromLocCode("P1-C40", 0, false))
+ .Times(0);
+ EXPECT_CALL(dataIface, getInventoryFromLocCode("P1", 0, false)).Times(0);
+
+ std::map<std::string, std::string> items{
+ {"CALLOUT_ERRNO", "5"},
+ {"CALLOUT_DEVICE_PATH",
+ "/sys/devices/platform/ahb/ahb:apb/ahb:apb:bus@1e78a000/1e78a340.i2c-bus/i2c-14/14-0072"}};
+
+ AdditionalData ad{items};
+ SRC src{entry, ad, dataIface};
+
+ ASSERT_TRUE(src.callouts());
+ auto& callouts = src.callouts()->callouts();
+
+ ASSERT_EQ(callouts.size(), 2);
+
+ // Should just contain the unexpanded location codes
+ {
+ EXPECT_EQ(callouts[0]->priority(), 'H');
+ EXPECT_EQ(callouts[0]->locationCode(), "P1-C40");
+
+ auto& fru = callouts[0]->fruIdentity();
+ EXPECT_EQ(fru->getPN().value(), "");
+ EXPECT_EQ(fru->getCCIN().value(), "");
+ EXPECT_EQ(fru->getSN().value(), "");
+ }
+ {
+ EXPECT_EQ(callouts[1]->priority(), 'M');
+ EXPECT_EQ(callouts[1]->locationCode(), "P1");
+
+ auto& fru = callouts[1]->fruIdentity();
+ EXPECT_EQ(fru->getPN().value(), "");
+ EXPECT_EQ(fru->getCCIN().value(), "");
+ EXPECT_EQ(fru->getSN().value(), "");
+ }
fs::remove_all(dataPath);
}
@@ -1249,7 +1451,8 @@
}
// Callout 1 mock calls
- // getInventoryFromLocCode will fail
+ // getInventoryFromLocCode will fail, so a callout with just the
+ // location code will be created.
{
EXPECT_CALL(dataIface, expandLocationCode("P0-C2", 0))
.Times(1)
@@ -1266,26 +1469,34 @@
const auto& callouts = src.callouts()->callouts();
- // Only the first callout was successful
- ASSERT_EQ(callouts.size(), 1);
+ // The first callout will have the unexpanded location code.
+ ASSERT_EQ(callouts.size(), 2);
- {
- EXPECT_EQ(callouts[0]->priority(), 'H');
- EXPECT_EQ(callouts[0]->locationCode(), "P0-C1");
+ EXPECT_EQ(callouts[0]->priority(), 'H');
+ EXPECT_EQ(callouts[0]->locationCode(), "P0-C1");
- auto& fru = callouts[0]->fruIdentity();
- EXPECT_EQ(fru->getPN().value(), "1234567");
- EXPECT_EQ(fru->getCCIN().value(), "CCCC");
- EXPECT_EQ(fru->getSN().value(), "123456789ABC");
- EXPECT_EQ(fru->failingComponentType(), src::FRUIdentity::hardwareFRU);
- }
+ auto& fru0 = callouts[0]->fruIdentity();
+ EXPECT_EQ(fru0->getPN().value(), "1234567");
+ EXPECT_EQ(fru0->getCCIN().value(), "CCCC");
+ EXPECT_EQ(fru0->getSN().value(), "123456789ABC");
+ EXPECT_EQ(fru0->failingComponentType(), src::FRUIdentity::hardwareFRU);
+
+ // The second callout will have empty HW details.
+ EXPECT_EQ(callouts[1]->priority(), 'H');
+ EXPECT_EQ(callouts[1]->locationCode(), "UXXX-P0-C2");
+
+ auto& fru1 = callouts[1]->fruIdentity();
+ EXPECT_EQ(fru1->getPN().value(), "");
+ EXPECT_EQ(fru1->getCCIN().value(), "");
+ EXPECT_EQ(fru1->getSN().value(), "");
+ EXPECT_EQ(fru1->failingComponentType(), src::FRUIdentity::hardwareFRU);
const auto& data = src.getDebugData();
ASSERT_EQ(data.size(), 4);
EXPECT_STREQ(data[0].c_str(), "Unable to expand location code P0-C1: Fail");
- EXPECT_STREQ(data[1].c_str(),
- "Failed extracting callout data from JSON: Unable to "
- "get inventory path from location code: P0-C2: Fail");
+ EXPECT_STREQ(
+ data[1].c_str(),
+ "Unable to get inventory path from location code: P0-C2: Fail");
EXPECT_STREQ(data[2].c_str(),
"Failed extracting callout data from JSON: "
"[json.exception.out_of_range.403] key 'Priority' not found");