Refactor singleton and filesystem path handling
- Remove singleton PostCodeDataHolder. It only contained a single int
which could easily be held inside the main PostCode class instead.
- Simplify D-Bus match construction by using predefined
PropertiesChanged rule.
- Remove unnecessary PostCode members which were just copies of const
strings.
- Refactor some filesystem path construction/handling to simplify code.
Tested:
Rebooted host a few times and observed that correct POST code history is
still populated in Redfish.
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
Change-Id: Ifd61751807da704eaf2a64dac34ca708fd28c872
diff --git a/inc/post_code.hpp b/inc/post_code.hpp
index 440e56f..243feaf 100644
--- a/inc/post_code.hpp
+++ b/inc/post_code.hpp
@@ -40,29 +40,12 @@
const static constexpr char* CurrentBootCycleIndexName =
"CurrentBootCycleIndex";
-// Singleton holder to store host/node and other path information
-class PostCodeDataHolder
-{
- PostCodeDataHolder() {}
-
- public:
- static PostCodeDataHolder& getInstance()
- {
- static PostCodeDataHolder instance;
- return instance;
- }
-
- int node;
-
- const static constexpr char* PostCodePath =
- "/xyz/openbmc_project/state/boot/raw";
- const static constexpr char* PropertiesIntf =
- "org.freedesktop.DBus.Properties";
- const static constexpr char* PostCodeListPathPrefix =
- "/var/lib/phosphor-post-code-manager/host";
- const static constexpr char* HostStatePathPrefix =
- "/xyz/openbmc_project/state/host";
-};
+const static constexpr char* PostCodePath =
+ "/xyz/openbmc_project/state/boot/raw";
+const static constexpr char* PostCodeListPathPrefix =
+ "/var/lib/phosphor-post-code-manager/host";
+const static constexpr char* HostStatePathPrefix =
+ "/xyz/openbmc_project/state/host";
struct EventDeleter
{
@@ -85,69 +68,55 @@
struct PostCode : sdbusplus::server::object_t<post_code, delete_all>
{
- PostCodeDataHolder& postcodeDataHolderObj =
- PostCodeDataHolder::getInstance();
-
- PostCode(sdbusplus::bus_t& bus, const char* path, EventPtr& /*event*/) :
+ PostCode(sdbusplus::bus_t& bus, const char* path, int nodeIndex) :
sdbusplus::server::object_t<post_code, delete_all>(bus, path), bus(bus),
+ node(nodeIndex),
+ postCodeListPath(PostCodeListPathPrefix + std::to_string(node)),
propertiesChangedSignalRaw(
bus,
- sdbusplus::bus::match::rules::type::signal() +
- sdbusplus::bus::match::rules::member("PropertiesChanged") +
- sdbusplus::bus::match::rules::path(
- postcodeDataHolderObj.PostCodePath +
- std::to_string(postcodeDataHolderObj.node)) +
- sdbusplus::bus::match::rules::interface(
- postcodeDataHolderObj.PropertiesIntf),
+ sdbusplus::bus::match::rules::propertiesChanged(
+ PostCodePath + std::to_string(node),
+ "xyz.openbmc_project.State.Boot.Raw"),
[this](sdbusplus::message_t& msg) {
- std::string objectName;
+ std::string intfName;
std::map<std::string, std::variant<postcode_t>> msgData;
- msg.read(objectName, msgData);
+ msg.read(intfName, msgData);
// Check if it was the Value property that changed.
auto valPropMap = msgData.find("Value");
+ if (valPropMap != msgData.end())
{
- if (valPropMap != msgData.end())
- {
- this->savePostCodes(
- std::get<postcode_t>(valPropMap->second));
- }
+ this->savePostCodes(
+ std::get<postcode_t>(valPropMap->second));
}
}),
propertiesChangedSignalCurrentHostState(
bus,
- sdbusplus::bus::match::rules::type::signal() +
- sdbusplus::bus::match::rules::member("PropertiesChanged") +
- sdbusplus::bus::match::rules::path(
- postcodeDataHolderObj.HostStatePathPrefix +
- std::to_string(postcodeDataHolderObj.node)) +
- sdbusplus::bus::match::rules::interface(
- postcodeDataHolderObj.PropertiesIntf),
+ sdbusplus::bus::match::rules::propertiesChanged(
+ HostStatePathPrefix + std::to_string(node),
+ "xyz.openbmc_project.State.Host"),
[this](sdbusplus::message_t& msg) {
- std::string objectName;
+ std::string intfName;
std::map<std::string, std::variant<std::string>> msgData;
- msg.read(objectName, msgData);
+ msg.read(intfName, msgData);
// Check if it was the Value property that changed.
auto valPropMap = msgData.find("CurrentHostState");
+ if (valPropMap != msgData.end())
{
- if (valPropMap != msgData.end())
+ StateServer::Host::HostState currentHostState =
+ StateServer::Host::convertHostStateFromString(
+ std::get<std::string>(valPropMap->second));
+ if (currentHostState == StateServer::Host::HostState::Off)
{
- StateServer::Host::HostState currentHostState =
- StateServer::Host::convertHostStateFromString(
- std::get<std::string>(valPropMap->second));
- if (currentHostState ==
- StateServer::Host::HostState::Off)
+ if (this->postCodes.empty())
{
- if (this->postCodes.empty())
- {
- std::cerr << "HostState changed to OFF. Empty "
- "postcode log, keep boot cycle at "
- << this->currentBootCycleIndex
- << std::endl;
- }
- else
- {
- this->postCodes.clear();
- }
+ std::cerr << "HostState changed to OFF. Empty "
+ "postcode log, keep boot cycle at "
+ << this->currentBootCycleIndex
+ << std::endl;
+ }
+ else
+ {
+ this->postCodes.clear();
}
}
}
@@ -155,22 +124,11 @@
{
phosphor::logging::log<phosphor::logging::level::INFO>(
"PostCode is created");
- auto dir = fs::path(postcodeDataHolderObj.PostCodeListPathPrefix +
- std::to_string(postcodeDataHolderObj.node));
- fs::create_directories(dir);
- strPostCodeListPath = postcodeDataHolderObj.PostCodeListPathPrefix +
- std::to_string(postcodeDataHolderObj.node) + "/";
- strCurrentBootCycleIndexName = CurrentBootCycleIndexName;
- uint16_t index = 0;
- deserialize(
- fs::path(strPostCodeListPath + strCurrentBootCycleIndexName),
- index);
- currentBootCycleIndex = index;
- strCurrentBootCycleCountName = CurrentBootCycleCountName;
+ fs::create_directories(postCodeListPath);
+ deserialize(postCodeListPath / CurrentBootCycleIndexName,
+ currentBootCycleIndex);
uint16_t count = 0;
- deserialize(
- fs::path(strPostCodeListPath + strCurrentBootCycleCountName),
- count);
+ deserialize(postCodeListPath / CurrentBootCycleCountName, count);
currentBootCycleCount(count);
maxBootCycleNum(MAX_BOOT_CYCLE_COUNT);
}
@@ -186,17 +144,17 @@
uint16_t getBootNum(const uint16_t index) const;
sdbusplus::bus_t& bus;
+ int node;
std::chrono::time_point<std::chrono::steady_clock> firstPostCodeTimeSteady;
uint64_t firstPostCodeUsSinceEpoch;
std::map<uint64_t, postcode_t> postCodes;
- std::string strPostCodeListPath;
- std::string strCurrentBootCycleIndexName;
- uint16_t currentBootCycleIndex;
- std::string strCurrentBootCycleCountName;
- void savePostCodes(postcode_t code);
+ fs::path postCodeListPath;
+ uint16_t currentBootCycleIndex = 0;
sdbusplus::bus::match_t propertiesChangedSignalRaw;
sdbusplus::bus::match_t propertiesChangedSignalCurrentHostState;
- fs::path serialize(const std::string& path);
+
+ void savePostCodes(postcode_t code);
+ fs::path serialize(const fs::path& path);
bool deserialize(const fs::path& path, uint16_t& index);
bool deserializePostCodes(const fs::path& path,
std::map<uint64_t, postcode_t>& codes);
diff --git a/src/main.cpp b/src/main.cpp
index 98e09fb..6d53629 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -19,12 +19,10 @@
int main(int argc, char* argv[])
{
- PostCodeDataHolder& postcodeDataHolderObj =
- PostCodeDataHolder::getInstance();
-
int arg;
int optIndex = 0;
int ret = 0;
+ int node = 0;
std::string intfName;
@@ -36,7 +34,7 @@
switch (arg)
{
case 'h':
- postcodeDataHolderObj.node = std::stoi(optarg);
+ node = std::stoi(optarg);
break;
default:
break;
@@ -59,15 +57,14 @@
sdbusplus::bus_t bus = sdbusplus::bus::new_default();
- std::string dbusObjName =
- DBUS_OBJECT_NAME + std::to_string(postcodeDataHolderObj.node);
+ std::string dbusObjName = DBUS_OBJECT_NAME + std::to_string(node);
sdbusplus::server::manager_t m{bus, dbusObjName.c_str()};
- intfName = DBUS_INTF_NAME + std::to_string(postcodeDataHolderObj.node);
+ intfName = DBUS_INTF_NAME + std::to_string(node);
bus.request_name(intfName.c_str());
- PostCode postCode{bus, dbusObjName.c_str(), eventP};
+ PostCode postCode{bus, dbusObjName.c_str(), node};
try
{
diff --git a/src/post_code.cpp b/src/post_code.cpp
index 82dba46..6186f97 100644
--- a/src/post_code.cpp
+++ b/src/post_code.cpp
@@ -15,18 +15,14 @@
*/
#include "post_code.hpp"
-#include "iomanip"
+#include <iomanip>
void PostCode::deleteAll()
{
- auto dir = fs::path(postcodeDataHolderObj.PostCodeListPathPrefix +
- std::to_string(postcodeDataHolderObj.node));
- std::uintmax_t n = fs::remove_all(dir);
+ std::uintmax_t n = fs::remove_all(postCodeListPath);
std::cerr << "clearPostCodes deleted " << n << " files in "
- << postcodeDataHolderObj.PostCodeListPathPrefix +
- std::to_string(postcodeDataHolderObj.node)
- << std::endl;
- fs::create_directories(dir);
+ << postCodeListPath << std::endl;
+ fs::create_directories(postCodeListPath);
postCodes.clear();
currentBootCycleIndex = 0;
currentBootCycleCount(0);
@@ -46,8 +42,7 @@
uint16_t bootNum = getBootNum(index);
decltype(postCodes) codes;
- deserializePostCodes(
- fs::path(strPostCodeListPath + std::to_string(bootNum)), codes);
+ deserializePostCodes(postCodeListPath / std::to_string(bootNum), codes);
std::transform(codes.begin(), codes.end(), std::back_inserter(codesVec),
[](const auto& kv) { return kv.second; });
}
@@ -64,8 +59,7 @@
uint16_t bootNum = getBootNum(index);
decltype(postCodes) codes;
- deserializePostCodes(
- fs::path(strPostCodeListPath + std::to_string(bootNum)), codes);
+ deserializePostCodes(postCodeListPath / std::to_string(bootNum), codes);
return codes;
}
@@ -97,7 +91,7 @@
{
postCodes.erase(postCodes.begin());
}
- serialize(fs::path(strPostCodeListPath));
+ serialize(postCodeListPath);
#ifdef ENABLE_BIOS_POST_CODE_LOG
uint64_t usTimeOffset = tsUS - firstPostCodeUsSinceEpoch;
@@ -125,23 +119,20 @@
return;
}
-fs::path PostCode::serialize(const std::string& path)
+fs::path PostCode::serialize(const fs::path& path)
{
try
{
- fs::path idxPath(path + strCurrentBootCycleIndexName);
- std::ofstream osIdx(idxPath.c_str(), std::ios::binary);
+ std::ofstream osIdx(path / CurrentBootCycleIndexName, std::ios::binary);
cereal::JSONOutputArchive idxArchive(osIdx);
idxArchive(currentBootCycleIndex);
uint16_t count = currentBootCycleCount();
- fs::path cntPath(path + strCurrentBootCycleCountName);
- std::ofstream osCnt(cntPath.c_str(), std::ios::binary);
+ std::ofstream osCnt(path / CurrentBootCycleCountName, std::ios::binary);
cereal::JSONOutputArchive cntArchive(osCnt);
cntArchive(count);
- std::ofstream osPostCodes(
- (path + std::to_string(currentBootCycleIndex)));
+ std::ofstream osPostCodes(path / std::to_string(currentBootCycleIndex));
cereal::JSONOutputArchive oarchivePostCodes(osPostCodes);
oarchivePostCodes(postCodes);
}
@@ -164,7 +155,7 @@
{
if (fs::exists(path))
{
- std::ifstream is(path.c_str(), std::ios::in | std::ios::binary);
+ std::ifstream is(path, std::ios::in | std::ios::binary);
cereal::JSONInputArchive iarchive(is);
iarchive(index);
return true;
@@ -191,7 +182,7 @@
{
if (fs::exists(path))
{
- std::ifstream is(path.c_str(), std::ios::in | std::ios::binary);
+ std::ifstream is(path, std::ios::in | std::ios::binary);
cereal::JSONInputArchive iarchive(is);
iarchive(codes);
return true;