Add Configuration class
Adding this class improves maintainability by increasing separation
of concern. One major improvement is the removal of reoccuring calls to
loadConfigurations. These calls took a long time. Now it's only called
once.
Tested:
QEMU/yosemite4 with probe statement set to TRUE.
```
root@yosemite4:~# journalctl | grep entity-manager
Dec 19 13:26:21 yosemite4 entity-manager[502]: Clearing previous configuration
Dec 19 13:26:25 yosemite4 entity-manager[502]: Inventory Added: Yosemite 4 Management Board
root@yosemite4:~# busctl tree xyz.openbmc_project.EntityManager
`- /xyz
`- /xyz/openbmc_project
|- /xyz/openbmc_project/EntityManager
`- /xyz/openbmc_project/inventory
`- /xyz/openbmc_project/inventory/system
`- /xyz/openbmc_project/inventory/system/board
`- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/All_Fan
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P0V6_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P12V_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P1V0_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P1V2_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P1V8_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P2V5_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P3V3_RGM_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P3V3_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P3V_BAT_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P5V_USB_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_ADC_P5V_VOLT_V
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/MGNT_TEMP_C
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/PID_NIC_TEMP
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/Stepwise_MGNT_TEMP
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/Stepwise_NIC_TEMP
|- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/Stepwise_SENTINEL_DOME_SLOT_PRESENT_PERCENTAGE
`- /xyz/openbmc_project/inventory/system/board/Yosemite_4_Management_Board/Zone_1
```
Change-Id: I5684a03232012e14d97edcf4ea5ed1aed4d50c8d
Signed-off-by: Christopher Meis <christopher.meis@9elements.com>
diff --git a/src/entity_manager/configuration.cpp b/src/entity_manager/configuration.cpp
index 137f4e3..9463133 100644
--- a/src/entity_manager/configuration.cpp
+++ b/src/entity_manager/configuration.cpp
@@ -13,28 +13,16 @@
#include <filesystem>
#include <fstream>
#include <iostream>
-#include <list>
#include <string>
#include <vector>
-namespace configuration
+Configuration::Configuration()
{
-// writes output files to persist data
-bool writeJsonFiles(const nlohmann::json& systemConfiguration)
-{
- std::filesystem::create_directory(configurationOutDir);
- std::ofstream output(currentConfiguration);
- if (!output.good())
- {
- return false;
- }
- output << systemConfiguration.dump(4);
- output.close();
- return true;
+ loadConfigurations();
+ filterProbeInterfaces();
}
-// reads json files out of the filesystem
-bool loadConfigurations(std::list<nlohmann::json>& configurations)
+void Configuration::loadConfigurations()
{
const auto start = std::chrono::steady_clock::now();
@@ -47,7 +35,7 @@
{
std::cerr << "Unable to find any configuration files in "
<< configurationDirectory << "\n";
- return false;
+ return;
}
std::ifstream schemaStream(
@@ -57,7 +45,7 @@
std::cerr
<< "Cannot open schema file, cannot validate JSON, exiting\n\n";
std::exit(EXIT_FAILURE);
- return false;
+ return;
}
nlohmann::json schema =
nlohmann::json::parse(schemaStream, nullptr, false, true);
@@ -66,7 +54,7 @@
std::cerr
<< "Illegal schema file detected, cannot validate JSON, exiting\n";
std::exit(EXIT_FAILURE);
- return false;
+ return;
}
for (auto& jsonPath : jsonPaths)
@@ -109,8 +97,6 @@
lg2::debug("Finished loading json configuration in {MILLIS}ms", "MILLIS",
duration);
-
- return true;
}
// Iterate over new configuration and erase items from old configuration.
@@ -144,15 +130,8 @@
}
// Extract the D-Bus interfaces to probe from the JSON config files.
-std::set<std::string> getProbeInterfaces()
+void Configuration::filterProbeInterfaces()
{
- std::set<std::string> interfaces;
- std::list<nlohmann::json> configurations;
- if (!configuration::loadConfigurations(configurations))
- {
- return interfaces;
- }
-
for (auto it = configurations.begin(); it != configurations.end();)
{
auto findProbe = it->find("Probe");
@@ -193,13 +172,22 @@
if (findStart != std::string::npos)
{
std::string interface = probe->substr(0, findStart);
- interfaces.emplace(interface);
+ probeInterfaces.emplace(interface);
}
}
it++;
}
-
- return interfaces;
}
-} // namespace configuration
+bool writeJsonFiles(const nlohmann::json& systemConfiguration)
+{
+ std::filesystem::create_directory(configurationOutDir);
+ std::ofstream output(currentConfiguration);
+ if (!output.good())
+ {
+ return false;
+ }
+ output << systemConfiguration.dump(4);
+ output.close();
+ return true;
+}
diff --git a/src/entity_manager/configuration.hpp b/src/entity_manager/configuration.hpp
index 015f713..f7f0b31 100644
--- a/src/entity_manager/configuration.hpp
+++ b/src/entity_manager/configuration.hpp
@@ -2,20 +2,28 @@
#include <nlohmann/json.hpp>
-#include <list>
-#include <set>
+#include <unordered_set>
+#include <vector>
-namespace configuration
-{
constexpr const char* globalSchema = "global.json";
constexpr const char* hostConfigurationDirectory = SYSCONF_DIR "configurations";
constexpr const char* configurationDirectory = PACKAGE_DIR "configurations";
constexpr const char* currentConfiguration = "/var/configuration/system.json";
constexpr const char* schemaDirectory = PACKAGE_DIR "schemas";
-bool writeJsonFiles(const nlohmann::json& systemConfiguration);
+class Configuration
+{
+ public:
+ explicit Configuration();
+ std::unordered_set<std::string> probeInterfaces;
+ std::vector<nlohmann::json> configurations;
-bool loadConfigurations(std::list<nlohmann::json>& configurations);
+ private:
+ void loadConfigurations();
+ void filterProbeInterfaces();
+};
+
+bool writeJsonFiles(const nlohmann::json& systemConfiguration);
template <typename JsonType>
bool setJsonFromPointer(const std::string& ptrStr, const JsonType& value,
@@ -39,7 +47,3 @@
bool validateJson(const nlohmann::json& schemaFile,
const nlohmann::json& input);
-
-std::set<std::string> getProbeInterfaces();
-
-} // namespace configuration
diff --git a/src/entity_manager/dbus_interface.cpp b/src/entity_manager/dbus_interface.cpp
index 1efa0c3..cea5d42 100644
--- a/src/entity_manager/dbus_interface.cpp
+++ b/src/entity_manager/dbus_interface.cpp
@@ -89,7 +89,7 @@
objServer.remove_interface(dbusInterface);
});
- if (!configuration::writeJsonFiles(systemConfiguration))
+ if (!writeJsonFiles(systemConfiguration))
{
std::cerr << "error setting json file\n";
throw DBusInternalError();
@@ -312,9 +312,8 @@
lastIndex++;
}
- std::ifstream schemaFile(
- std::string(configuration::schemaDirectory) + "/" +
- boost::to_lower_copy(*type) + ".json");
+ std::ifstream schemaFile(std::string(schemaDirectory) + "/" +
+ boost::to_lower_copy(*type) + ".json");
// todo(james) we might want to also make a list of 'can add'
// interfaces but for now I think the assumption if there is a
// schema avaliable that it is allowed to update is fine
@@ -330,7 +329,7 @@
std::cerr << "Schema not legal" << *type << ".json\n";
throw DBusInternalError();
}
- if (!configuration::validateJson(schema, newData))
+ if (!validateJson(schema, newData))
{
throw std::invalid_argument("Data does not match schema");
}
@@ -342,7 +341,7 @@
{
findExposes->push_back(newData);
}
- if (!configuration::writeJsonFiles(systemConfiguration))
+ if (!writeJsonFiles(systemConfiguration))
{
std::cerr << "Error writing json files\n";
throw DBusInternalError();
diff --git a/src/entity_manager/dbus_interface.hpp b/src/entity_manager/dbus_interface.hpp
index 51d967e..84efec8 100644
--- a/src/entity_manager/dbus_interface.hpp
+++ b/src/entity_manager/dbus_interface.hpp
@@ -69,13 +69,13 @@
const std::vector<PropertyType>& newVal,
std::vector<PropertyType>& val) {
val = newVal;
- if (!configuration::setJsonFromPointer(jsonPointerString, val,
- systemConfiguration))
+ if (!setJsonFromPointer(jsonPointerString, val,
+ systemConfiguration))
{
std::cerr << "error setting json field\n";
return -1;
}
- if (!configuration::writeJsonFiles(systemConfiguration))
+ if (!writeJsonFiles(systemConfiguration))
{
std::cerr << "error setting json file\n";
return -1;
@@ -103,13 +103,13 @@
jsonPointerString{std::string(jsonPointerString)}](
const PropertyType& newVal, PropertyType& val) {
val = newVal;
- if (!configuration::setJsonFromPointer(jsonPointerString, val,
- systemConfiguration))
+ if (!setJsonFromPointer(jsonPointerString, val,
+ systemConfiguration))
{
std::cerr << "error setting json field\n";
return -1;
}
- if (!configuration::writeJsonFiles(systemConfiguration))
+ if (!writeJsonFiles(systemConfiguration))
{
std::cerr << "error setting json file\n";
return -1;
diff --git a/src/entity_manager/entity_manager.cpp b/src/entity_manager/entity_manager.cpp
index 31e7ef4..ff9a5d5 100644
--- a/src/entity_manager/entity_manager.cpp
+++ b/src/entity_manager/entity_manager.cpp
@@ -86,6 +86,8 @@
propertiesChangedCallback();
});
dbus_interface::tryIfaceInitialize(entityIface);
+
+ initFilters(configuration.probeInterfaces);
}
void EntityManager::postToDbus(const nlohmann::json& newConfiguration)
@@ -450,7 +452,7 @@
loadOverlays(newConfiguration, io);
boost::asio::post(io, [this]() {
- if (!configuration::writeJsonFiles(systemConfiguration))
+ if (!writeJsonFiles(systemConfiguration))
{
std::cerr << "Error writing json files\n";
}
@@ -498,16 +500,8 @@
auto missingConfigurations = std::make_shared<nlohmann::json>();
*missingConfigurations = systemConfiguration;
- std::list<nlohmann::json> configurations;
- if (!configuration::loadConfigurations(configurations))
- {
- std::cerr << "Could not load configurations\n";
- propertiesChangedInProgress = false;
- return;
- }
-
auto perfScan = std::make_shared<scan::PerformScan>(
- *this, *missingConfigurations, configurations, io,
+ *this, *missingConfigurations, configuration.configurations, io,
[this, count, oldConfiguration, missingConfigurations]() {
// this is something that since ac has been applied to the
// bmc we saw, and we no longer see it
@@ -517,11 +511,9 @@
{
pruneConfiguration(powerOff, name, device);
}
-
nlohmann::json newConfiguration = systemConfiguration;
- configuration::deriveNewConfiguration(oldConfiguration,
- newConfiguration);
+ deriveNewConfiguration(oldConfiguration, newConfiguration);
for (const auto& [_, device] : newConfiguration.items())
{
@@ -542,7 +534,8 @@
// Check if InterfacesAdded payload contains an iface that needs probing.
static bool iaContainsProbeInterface(
- sdbusplus::message_t& msg, const std::set<std::string>& probeInterfaces)
+ sdbusplus::message_t& msg,
+ const std::unordered_set<std::string>& probeInterfaces)
{
sdbusplus::message::object_path path;
DBusObject interfaces;
@@ -564,7 +557,8 @@
// Check if InterfacesRemoved payload contains an iface that needs probing.
static bool irContainsProbeInterface(
- sdbusplus::message_t& msg, const std::set<std::string>& probeInterfaces)
+ sdbusplus::message_t& msg,
+ const std::unordered_set<std::string>& probeInterfaces)
{
sdbusplus::message::object_path path;
std::set<std::string> interfaces;
@@ -582,15 +576,13 @@
{
if (em_utils::fwVersionIsSame())
{
- if (std::filesystem::is_regular_file(
- configuration::currentConfiguration))
+ if (std::filesystem::is_regular_file(currentConfiguration))
{
// this file could just be deleted, but it's nice for debug
std::filesystem::create_directory(tempConfigDir);
std::filesystem::remove(lastConfiguration);
- std::filesystem::copy(configuration::currentConfiguration,
- lastConfiguration);
- std::filesystem::remove(configuration::currentConfiguration);
+ std::filesystem::copy(currentConfiguration, lastConfiguration);
+ std::filesystem::remove(currentConfiguration);
std::ifstream jsonStream(lastConfiguration);
if (jsonStream.good())
@@ -616,7 +608,7 @@
{
// not an error, just logging at this level to make it in the journal
std::cerr << "Clearing previous configuration\n";
- std::filesystem::remove(configuration::currentConfiguration);
+ std::filesystem::remove(currentConfiguration);
}
}
@@ -644,7 +636,8 @@
// org.freedesktop.DBus.Properties signals. Similarly if a process exits
// for any reason, expected or otherwise, we'll need a poke to remove
// entities from DBus.
-void EntityManager::initFilters(const std::set<std::string>& probeInterfaces)
+void EntityManager::initFilters(
+ const std::unordered_set<std::string>& probeInterfaces)
{
nameOwnerChangedMatch = std::make_unique<sdbusplus::bus::match_t>(
static_cast<sdbusplus::bus_t&>(*systemBus),
@@ -694,10 +687,6 @@
nlohmann::json systemConfiguration = nlohmann::json::object();
- std::set<std::string> probeInterfaces = configuration::getProbeInterfaces();
-
- em.initFilters(probeInterfaces);
-
boost::asio::post(io, [&]() { em.propertiesChangedCallback(); });
em.handleCurrentConfigurationJson();
diff --git a/src/entity_manager/entity_manager.hpp b/src/entity_manager/entity_manager.hpp
index cbb5f0a..c244ad3 100644
--- a/src/entity_manager/entity_manager.hpp
+++ b/src/entity_manager/entity_manager.hpp
@@ -18,6 +18,7 @@
#pragma once
#include "../utils.hpp"
+#include "configuration.hpp"
#include "dbus_interface.hpp"
#include "topology.hpp"
@@ -50,6 +51,7 @@
std::shared_ptr<sdbusplus::asio::connection> systemBus;
sdbusplus::asio::object_server objServer;
std::shared_ptr<sdbusplus::asio::dbus_interface> entityIface;
+ Configuration configuration;
nlohmann::json lastJson;
nlohmann::json systemConfiguration;
Topology topology;
@@ -66,8 +68,6 @@
void pruneConfiguration(bool powerOff, const std::string& name,
const nlohmann::json& device);
- void initFilters(const std::set<std::string>& probeInterfaces);
-
void handleCurrentConfigurationJson();
private:
@@ -87,6 +87,7 @@
void startRemovedTimer(boost::asio::steady_timer& timer,
nlohmann::json& systemConfiguration);
+ void initFilters(const std::unordered_set<std::string>& probeInterfaces);
};
inline void logDeviceAdded(const nlohmann::json& record)
diff --git a/src/entity_manager/perform_scan.cpp b/src/entity_manager/perform_scan.cpp
index 5e56f9c..c3e5310 100644
--- a/src/entity_manager/perform_scan.cpp
+++ b/src/entity_manager/perform_scan.cpp
@@ -196,7 +196,7 @@
scan::PerformScan::PerformScan(
EntityManager& em, nlohmann::json& missingConfigurations,
- std::list<nlohmann::json>& configurations, boost::asio::io_context& io,
+ std::vector<nlohmann::json>& configurations, boost::asio::io_context& io,
std::function<void()>&& callback) :
_em(em), _missingConfigurations(missingConfigurations),
_configurations(configurations), _callback(std::move(callback)), io(io)
diff --git a/src/entity_manager/perform_scan.hpp b/src/entity_manager/perform_scan.hpp
index 17396d4..b482d7b 100644
--- a/src/entity_manager/perform_scan.hpp
+++ b/src/entity_manager/perform_scan.hpp
@@ -26,7 +26,7 @@
struct PerformScan : std::enable_shared_from_this<PerformScan>
{
PerformScan(EntityManager& em, nlohmann::json& missingConfigurations,
- std::list<nlohmann::json>& configurations,
+ std::vector<nlohmann::json>& configurations,
boost::asio::io_context& io, std::function<void()>&& callback);
void updateSystemConfiguration(const nlohmann::json& recordRef,
@@ -36,7 +36,7 @@
virtual ~PerformScan();
EntityManager& _em;
nlohmann::json& _missingConfigurations;
- std::list<nlohmann::json> _configurations;
+ std::vector<nlohmann::json> _configurations;
std::function<void()> _callback;
bool _passed = false;
MapperGetSubTreeResponse dbusProbeObjects;