PEL: Remove 'no_vpd_for_fru' maintenance procedure
This procedure was being used in a callout when the location code could
not be found on an inventory item. The code was changed to just not add
a callout in this case.
This was done for the following reasons:
1) There's no expected reason that the inventory would be missing a
location code for a valid inventory path.
2) There wasn't a way to give a hint to the end user about what the
callout should be in that case. Usually maintenance procedures should
have steps one can take to still do the appropriate replacement.
3) Because of 1), this case indicates a bad inventory path passed in by
the caller. Theoretically there is a way to have a 'bmc_code'
callout, but that wouldn't be appropriate to add in this log since
the intent of this one is for another problem.
3a) There is no way to create a new error log from inside a PEL section,
and I don't think this one case warrants coming up with a way to do
so.
4) The inventory path in question is still being added into a UserData
section so that development can debug it.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I0757080f04942807cc34029d8667387db6b959fe
diff --git a/extensions/openpower-pels/pel_values.cpp b/extensions/openpower-pels/pel_values.cpp
index 11128bd..b98aebf 100644
--- a/extensions/openpower-pels/pel_values.cpp
+++ b/extensions/openpower-pels/pel_values.cpp
@@ -217,7 +217,7 @@
* to their actual names.
*/
const std::map<std::string, std::string> maintenanceProcedures = {
- {"no_vpd_for_fru", "BMCSP01"}, {"bmc_code", "BMCSP02"}};
+ {"bmc_code", "BMCSP01"}};
/**
* @brief Map of the registry names for the symbolic FRUs to their
diff --git a/extensions/openpower-pels/registry/schema/registry_example.json b/extensions/openpower-pels/registry/schema/registry_example.json
index 84071ec..fd2a1c0 100644
--- a/extensions/openpower-pels/registry/schema/registry_example.json
+++ b/extensions/openpower-pels/registry/schema/registry_example.json
@@ -63,7 +63,7 @@
[
{
"Priority": "high",
- "Procedure": "no_vpd_for_fru"
+ "Procedure": "bmc_code"
}
]
}
diff --git a/extensions/openpower-pels/registry/schema/schema.json b/extensions/openpower-pels/registry/schema/schema.json
index cbfb48a..48397a9 100644
--- a/extensions/openpower-pels/registry/schema/schema.json
+++ b/extensions/openpower-pels/registry/schema/schema.json
@@ -451,7 +451,7 @@
{
"description": "The maintenance procedure callout.",
"type": "string",
- "enum": ["no_vpd_for_fru", "bmc_code"]
+ "enum": ["bmc_code"]
},
"useInventoryLocCode":
diff --git a/extensions/openpower-pels/src.cpp b/extensions/openpower-pels/src.cpp
index b3e3e89..a6498af 100644
--- a/extensions/openpower-pels/src.cpp
+++ b/extensions/openpower-pels/src.cpp
@@ -878,12 +878,24 @@
": " + e.what();
addDebugData(msg);
- callout = std::make_unique<src::Callout>(CalloutPriority::high,
- "no_vpd_for_fru");
+ // Don't add a callout in this case, because:
+ // 1) With how the inventory is primed, there is no case where
+ // a location code is expected to be missing. This implies
+ // the caller is passing in something invalid.
+ // 2) The addDebugData call above will put the passed in path into
+ // a user data section that can be seen by development for debug.
+ // 3) Even if we wanted to do a 'no_vpd_for_fru' sort of maint.
+ // procedure, we don't have a good way to indicate to the user
+ // anything about the intended callout (they won't see user data).
+ // 4) Creating a new standalone event log for this problem isn't
+ // possible from inside a PEL section.
}
- createCalloutsObject();
- _callouts->addCallout(std::move(callout));
+ if (callout)
+ {
+ createCalloutsObject();
+ _callouts->addCallout(std::move(callout));
+ }
}
std::vector<message::RegistryCallout>
diff --git a/test/openpower-pels/fru_identity_test.cpp b/test/openpower-pels/fru_identity_test.cpp
index 116fe4d..4408b25 100644
--- a/test/openpower-pels/fru_identity_test.cpp
+++ b/test/openpower-pels/fru_identity_test.cpp
@@ -160,7 +160,7 @@
TEST(FRUIdentityTest, CreateProcedureCalloutTest)
{
{
- FRUIdentity fru{"no_vpd_for_fru"};
+ FRUIdentity fru{"bmc_code"};
EXPECT_EQ(fru.flattenedSize(), 12);
EXPECT_EQ(fru.type(), 0x4944);
diff --git a/test/openpower-pels/pel_manager_test.cpp b/test/openpower-pels/pel_manager_test.cpp
index c1cf5ec..32c765c 100644
--- a/test/openpower-pels/pel_manager_test.cpp
+++ b/test/openpower-pels/pel_manager_test.cpp
@@ -304,7 +304,7 @@
"00000000 00000000");
// Check if resolution property creation is good
EXPECT_EQ(manager.getResolution(pel),
- "1. Priority: High, Procedure: BMCSP02\n2. Priority: Medium, PN: "
+ "1. Priority: High, Procedure: BMCSP01\n2. Priority: Medium, PN: "
"SVCDOCS\n");
// Remove it
diff --git a/test/openpower-pels/registry_test.cpp b/test/openpower-pels/registry_test.cpp
index be23b82..c7d88c9 100644
--- a/test/openpower-pels/registry_test.cpp
+++ b/test/openpower-pels/registry_test.cpp
@@ -399,7 +399,7 @@
[
{
"Priority": "medium",
- "Procedure": "no_vpd_for_fru"
+ "Procedure": "bmc_code"
},
{
"Priority": "low",
@@ -444,7 +444,7 @@
EXPECT_EQ(callouts.size(), 2);
EXPECT_EQ(callouts[0].priority, "medium");
EXPECT_EQ(callouts[0].locCode, "");
- EXPECT_EQ(callouts[0].procedure, "no_vpd_for_fru");
+ EXPECT_EQ(callouts[0].procedure, "bmc_code");
EXPECT_EQ(callouts[0].symbolicFRU, "");
EXPECT_EQ(callouts[1].priority, "low");
EXPECT_EQ(callouts[1].locCode, "P3-C8");
@@ -557,7 +557,7 @@
},
{
"Priority": "low",
- "Procedure": "no_vpd_for_fru",
+ "Procedure": "bmc_code",
"CalloutType": "config_procedure"
}
]
@@ -612,7 +612,7 @@
EXPECT_EQ(callouts[1].symbolicFRUTrusted, "");
EXPECT_EQ(callouts[2].priority, "low");
EXPECT_EQ(callouts[2].locCode, "");
- EXPECT_EQ(callouts[2].procedure, "no_vpd_for_fru");
+ EXPECT_EQ(callouts[2].procedure, "bmc_code");
EXPECT_EQ(callouts[2].symbolicFRU, "");
EXPECT_EQ(callouts[2].symbolicFRUTrusted, "");
diff --git a/test/openpower-pels/src_callout_test.cpp b/test/openpower-pels/src_callout_test.cpp
index 12f8c2b..a5ca1ad 100644
--- a/test/openpower-pels/src_callout_test.cpp
+++ b/test/openpower-pels/src_callout_test.cpp
@@ -309,7 +309,7 @@
// Create a callout object by passing in the maintenance procedure to add.
TEST(CalloutTest, TestProcedureCallout)
{
- Callout callout{CalloutPriority::medium, "no_vpd_for_fru"};
+ Callout callout{CalloutPriority::medium, "bmc_code"};
// size/flags/pri/locsize fields + FRUIdentity size
// No location code.
diff --git a/test/openpower-pels/src_test.cpp b/test/openpower-pels/src_test.cpp
index 3149378..713eed5 100644
--- a/test/openpower-pels/src_test.cpp
+++ b/test/openpower-pels/src_test.cpp
@@ -393,7 +393,7 @@
}
// Test that when the location code can't be obtained that
-// a procedure callout is used.
+// no callout is added.
TEST_F(SRCTest, InventoryCalloutNoLocCodeTest)
{
message::Entry entry;
@@ -420,20 +420,7 @@
SRC src{entry, ad, dataIface};
EXPECT_TRUE(src.valid());
- ASSERT_TRUE(src.callouts());
-
- EXPECT_EQ(src.callouts()->callouts().size(), 1);
-
- auto& callout = src.callouts()->callouts().front();
- EXPECT_EQ(callout->locationCodeSize(), 0);
- EXPECT_EQ(callout->priority(), 'H');
-
- auto& fru = callout->fruIdentity();
-
- EXPECT_EQ(fru->getMaintProc().value(), "BMCSP01");
- EXPECT_FALSE(fru->getPN());
- EXPECT_FALSE(fru->getSN());
- EXPECT_FALSE(fru->getCCIN());
+ ASSERT_FALSE(src.callouts());
// flatten and unflatten
std::vector<uint8_t> data;
@@ -443,8 +430,7 @@
stream.offset(0);
SRC newSRC{stream};
EXPECT_TRUE(newSRC.valid());
- ASSERT_TRUE(src.callouts());
- EXPECT_EQ(src.callouts()->callouts().size(), 1);
+ ASSERT_FALSE(src.callouts());
}
// Test that when the VPD can't be obtained that
@@ -519,7 +505,7 @@
},
{
"Priority": "medium",
- "Procedure": "no_vpd_for_fru"
+ "Procedure": "bmc_code"
}
]
},