Make dbusconfiguration reloadable without reboot

Now that asio is being used instead of threads, we can
reload the fan configuration without having to restart the
application. This moves the ownership of the passive and host
bus outside of the SensorManager so that it can be recreated
each reload.

Tested: Watched logs and saw full fan config get reloaded
after changing fan configuration

Tested: Ran on json configured system and it behaved as
expected.

Change-Id: I00e6b27f75384fd41de2017b723f159c9691ae97
Signed-off-by: James Feist <james.feist@linux.intel.com>

10 files changed, 161 insertions, 112 deletions

diff --git a/dbus/dbusconfiguration.cpp b/dbus/dbusconfiguration.cpp
index 2c9bbc2..870e5e8 100644
--- a/dbus/dbusconfiguration.cpp
+++ b/dbus/dbusconfiguration.cpp
@@ -18,15 +18,16 @@
 #include "util.hpp"
 
 #include <algorithm>
+#include <boost/asio/steady_timer.hpp>
 #include <chrono>
 #include <functional>
 #include <iostream>
+#include <list>
 #include <regex>
 #include <sdbusplus/bus.hpp>
 #include <sdbusplus/bus/match.hpp>
 #include <sdbusplus/exception.hpp>
 #include <set>
-#include <thread>
 #include <unordered_map>
 #include <variant>
 
@@ -139,16 +140,6 @@ void debugPrint(void)
     std::cout << "}\n\n";
 }
 
-int eventHandler(sd_bus_message*, void*, sd_bus_error*)
-{
-    // do a brief sleep as we tend to get a bunch of these events at
-    // once
-    std::this_thread::sleep_for(std::chrono::seconds(5));
-    std::cout << "New configuration detected, restarting\n.";
-    std::exit(EXIT_SUCCESS); // service file should make us restart
-    return 1;
-}
-
 size_t getZoneIndex(const std::string& name, std::vector<std::string>& zones)
 {
     auto it = std::find(zones.begin(), zones.end(), name);
@@ -229,8 +220,74 @@ std::vector<std::string> getSelectedProfiles(sdbusplus::bus::bus& bus)
     return ret;
 }
 
-void init(sdbusplus::bus::bus& bus)
+int eventHandler(sd_bus_message*, void* context, sd_bus_error*)
+{
+
+    if (context == nullptr)
+    {
+        throw std::runtime_error("Invalid match");
+    }
+    boost::asio::steady_timer* timer =
+        static_cast<boost::asio::steady_timer*>(context);
+
+    // do a brief sleep as we tend to get a bunch of these events at
+    // once
+    timer->expires_after(std::chrono::seconds(2));
+    timer->async_wait([](const boost::system::error_code ec) {
+        if (ec == boost::asio::error::operation_aborted)
+        {
+            /* another timer started*/
+            return;
+        }
+
+        std::cout << "New configuration detected, reloading\n.";
+        restartControlLoops();
+    });
+
+    return 1;
+}
+
+void createMatches(sdbusplus::bus::bus& bus, boost::asio::steady_timer& timer)
+{
+    // this is a list because the matches can't be moved
+    static std::list<sdbusplus::bus::match::match> matches;
+
+    const std::array<std::string, 5> interfaces = {
+        thermalControlIface, fanProfileConfigurationIface,
+        pidConfigurationInterface, pidZoneConfigurationInterface,
+        stepwiseConfigurationInterface};
+
+    // this list only needs to be created once
+    if (!matches.empty())
+    {
+        return;
+    }
+
+    // we restart when the configuration changes or there are new sensors
+    for (const auto& interface : interfaces)
+    {
+        matches.emplace_back(
+            bus,
+            "type='signal',member='PropertiesChanged',arg0namespace='" +
+                interface + "'",
+            eventHandler, &timer);
+    }
+    matches.emplace_back(
+        bus,
+        "type='signal',member='InterfacesAdded',arg0path='/xyz/openbmc_project/"
+        "sensors/'",
+        eventHandler, &timer);
+}
+
+bool init(sdbusplus::bus::bus& bus, boost::asio::steady_timer& timer)
 {
+
+    sensorConfig.clear();
+    zoneConfig.clear();
+    zoneDetailsConfig.clear();
+
+    createMatches(bus, timer);
+
     using DbusVariantType =
         std::variant<uint64_t, int64_t, double, std::string,
                      std::vector<std::string>, std::vector<double>>;
@@ -240,27 +297,6 @@ void init(sdbusplus::bus::bus& bus)
         std::unordered_map<std::string,
                            std::unordered_map<std::string, DbusVariantType>>>;
 
-    // restart on configuration properties changed
-    static sdbusplus::bus::match::match configMatch(
-        bus,
-        "type='signal',member='PropertiesChanged',arg0namespace='" +
-            std::string(pidConfigurationInterface) + "'",
-        eventHandler);
-
-    // restart on profile change
-    static sdbusplus::bus::match::match profileMatch(
-        bus,
-        "type='signal',member='PropertiesChanged',arg0namespace='" +
-            std::string(thermalControlIface) + "'",
-        eventHandler);
-
-    // restart on sensors changed
-    static sdbusplus::bus::match::match sensorAdded(
-        bus,
-        "type='signal',member='InterfacesAdded',arg0path='/xyz/openbmc_project/"
-        "sensors/'",
-        eventHandler);
-
     auto mapper =
         bus.new_method_call("xyz.openbmc_project.ObjectMapper",
                             "/xyz/openbmc_project/object_mapper",
@@ -768,12 +804,10 @@ void init(sdbusplus::bus::bus& bus)
     }
     if (zoneConfig.empty() || zoneDetailsConfig.empty())
     {
-        std::cerr << "No fan zones, application pausing until reboot\n";
-        while (1)
-        {
-            bus.process_discard();
-            bus.wait();
-        }
+        std::cerr
+            << "No fan zones, application pausing until new configuration\n";
+        return false;
     }
+    return true;
 }
 } // namespace dbus_configuration
diff --git a/dbus/dbusconfiguration.hpp b/dbus/dbusconfiguration.hpp
index b3032e9..a9b12a9 100644
--- a/dbus/dbusconfiguration.hpp
+++ b/dbus/dbusconfiguration.hpp
@@ -21,5 +21,5 @@
 
 namespace dbus_configuration
 {
-void init(sdbusplus::bus::bus& bus);
+bool init(sdbusplus::bus::bus& bus, boost::asio::steady_timer& timer);
 } // namespace dbus_configuration
diff --git a/main.cpp b/main.cpp
index 8c99154..db355ab 100644
--- a/main.cpp
+++ b/main.cpp
@@ -48,47 +48,43 @@
 #include "dbus/dbusconfiguration.hpp"
 #endif
 
-/* The YAML converted sensor list. */
+/* The configuration converted sensor list. */
 std::map<std::string, struct conf::SensorConfig> sensorConfig = {};
-/* The YAML converted PID list. */
+/* The configuration converted PID list. */
 std::map<int64_t, conf::PIDConf> zoneConfig = {};
-/* The YAML converted Zone configuration. */
+/* The configuration converted Zone configuration. */
 std::map<int64_t, struct conf::ZoneConfig> zoneDetailsConfig = {};
 
 /** the swampd daemon will check for the existence of this file. */
 constexpr auto jsonConfigurationPath = "/usr/share/swampd/config.json";
+std::string configPath = "";
 
-int main(int argc, char* argv[])
-{
-    int rc = 0;
-    std::string configPath = "";
-    loggingPath = "";
-    loggingEnabled = false;
-    tuningEnabled = false;
-
-    CLI::App app{"OpenBMC Fan Control Daemon"};
-
-    app.add_option("-c,--conf", configPath,
-                   "Optional parameter to specify configuration at run-time")
-        ->check(CLI::ExistingFile);
-    app.add_option("-l,--log", loggingPath,
-                   "Optional parameter to specify logging folder")
-        ->check(CLI::ExistingDirectory);
-    app.add_flag("-t,--tuning", tuningEnabled, "Enable or disable tuning");
+/* async io context for operation */
+boost::asio::io_context io;
 
-    loggingEnabled = (!loggingPath.empty());
+/* buses for system control */
+static sdbusplus::asio::connection modeControlBus(io);
+static sdbusplus::asio::connection
+    hostBus(io, sdbusplus::bus::new_system().release());
+static sdbusplus::asio::connection
+    passiveBus(io, sdbusplus::bus::new_system().release());
 
-    CLI11_PARSE(app, argc, argv);
+void restartControlLoops()
+{
+    static SensorManager mgmr;
+    static std::unordered_map<int64_t, std::unique_ptr<PIDZone>> zones;
+    static std::list<boost::asio::steady_timer> timers;
 
-    auto modeControlBus = sdbusplus::bus::new_system();
-    static constexpr auto modeRoot = "/xyz/openbmc_project/settings/fanctrl";
-    // Create a manager for the ModeBus because we own it.
-    sdbusplus::server::manager::manager(modeControlBus, modeRoot);
+    timers.clear();
 
 #if CONFIGURE_DBUS
+
+    static boost::asio::steady_timer reloadTimer(io);
+    if (!dbus_configuration::init(modeControlBus, reloadTimer))
     {
-        dbus_configuration::init(modeControlBus);
+        return; // configuration not ready
     }
+
 #else
     const std::string& path =
         (configPath.length() > 0) ? configPath : jsonConfigurationPath;
@@ -110,42 +106,57 @@ int main(int argc, char* argv[])
     }
 #endif
 
-    SensorManager mgmr = buildSensors(sensorConfig);
-    std::unordered_map<int64_t, std::unique_ptr<PIDZone>> zones =
-        buildZones(zoneConfig, zoneDetailsConfig, mgmr, modeControlBus);
+    mgmr = buildSensors(sensorConfig, passiveBus, hostBus);
+    zones = buildZones(zoneConfig, zoneDetailsConfig, mgmr, modeControlBus);
 
     if (0 == zones.size())
     {
         std::cerr << "No zones defined, exiting.\n";
-        return rc;
+        std::exit(EXIT_FAILURE);
     }
 
-    /*
-     * All sensors are managed by one manager, but each zone has a pointer to
-     * it.
-     */
+    for (const auto& i : zones)
+    {
+        auto& timer = timers.emplace_back(io);
+        std::cerr << "pushing zone " << i.first << "\n";
+        pidControlLoop(i.second.get(), timer);
+    }
+}
 
-    auto& hostSensorBus = mgmr.getHostBus();
-    auto& passiveListeningBus = mgmr.getPassiveBus();
+int main(int argc, char* argv[])
+{
+    loggingPath = "";
+    loggingEnabled = false;
+    tuningEnabled = false;
 
-    boost::asio::io_context io;
-    sdbusplus::asio::connection passiveBus(io, passiveListeningBus.release());
+    CLI::App app{"OpenBMC Fan Control Daemon"};
 
-    sdbusplus::asio::connection hostBus(io, hostSensorBus.release());
-    hostBus.request_name("xyz.openbmc_project.Hwmon.external");
+    app.add_option("-c,--conf", configPath,
+                   "Optional parameter to specify configuration at run-time")
+        ->check(CLI::ExistingFile);
+    app.add_option("-l,--log", loggingPath,
+                   "Optional parameter to specify logging folder")
+        ->check(CLI::ExistingDirectory);
+    app.add_flag("-t,--tuning", tuningEnabled, "Enable or disable tuning");
 
-    sdbusplus::asio::connection modeBus(io, modeControlBus.release());
-    modeBus.request_name("xyz.openbmc_project.State.FanCtrl");
+    loggingEnabled = (!loggingPath.empty());
 
-    std::list<boost::asio::steady_timer> timers;
+    CLI11_PARSE(app, argc, argv);
 
-    for (const auto& i : zones)
-    {
-        auto& timer = timers.emplace_back(io);
-        std::cerr << "pushing zone" << std::endl;
-        pidControlLoop(i.second.get(), timer);
-    }
+    static constexpr auto modeRoot = "/xyz/openbmc_project/settings/fanctrl";
+    // Create a manager for the ModeBus because we own it.
+    sdbusplus::server::manager::manager(
+        static_cast<sdbusplus::bus::bus&>(modeControlBus), modeRoot);
+    hostBus.request_name("xyz.openbmc_project.Hwmon.external");
+    modeControlBus.request_name("xyz.openbmc_project.State.FanCtrl");
+
+    /*
+     * All sensors are managed by one manager, but each zone has a pointer to
+     * it.
+     */
+
+    restartControlLoops();
 
     io.run();
-    return rc;
+    return 0;
 }
diff --git a/pid/pidloop.cpp b/pid/pidloop.cpp
index f613c7e..9eed3a8 100644
--- a/pid/pidloop.cpp
+++ b/pid/pidloop.cpp
@@ -57,6 +57,11 @@ void pidControlLoop(PIDZone* zone, boost::asio::steady_timer& timer, bool first,
     timer.expires_after(std::chrono::milliseconds(100));
     timer.async_wait(
         [zone, &timer, ms100cnt](const boost::system::error_code& ec) mutable {
+            if (ec == boost::asio::error::operation_aborted)
+            {
+                return; // timer being canceled, stop loop
+            }
+
             /*
              * This should sleep on the conditional wait for the listen thread
              * to tell us it's in sync.  But then we also need a timeout option
diff --git a/sensors/builder.cpp b/sensors/builder.cpp
index 1a74adb..f5c01b3 100644
--- a/sensors/builder.cpp
+++ b/sensors/builder.cpp
@@ -38,9 +38,10 @@ static constexpr bool deferSignals = true;
 static DbusHelper helper;
 
 SensorManager
-    buildSensors(const std::map<std::string, struct conf::SensorConfig>& config)
+    buildSensors(const std::map<std::string, struct conf::SensorConfig>& config,
+                 sdbusplus::bus::bus& passive, sdbusplus::bus::bus& host)
 {
-    SensorManager mgmr;
+    SensorManager mgmr(passive, host);
     auto& hostSensorBus = mgmr.getHostBus();
     auto& passiveListeningBus = mgmr.getPassiveBus();
 
diff --git a/sensors/builder.hpp b/sensors/builder.hpp
index 224d467..688a931 100644
--- a/sensors/builder.hpp
+++ b/sensors/builder.hpp
@@ -9,5 +9,6 @@
 /**
  * Build the sensors and associate them with a SensorManager.
  */
-SensorManager buildSensors(
-    const std::map<std::string, struct conf::SensorConfig>& config);
+SensorManager
+    buildSensors(const std::map<std::string, struct conf::SensorConfig>& config,
+                 sdbusplus::bus::bus& passive, sdbusplus::bus::bus& host);
diff --git a/sensors/manager.hpp b/sensors/manager.hpp
index a6b4b9d..411b302 100644
--- a/sensors/manager.hpp
+++ b/sensors/manager.hpp
@@ -15,19 +15,14 @@
 class SensorManager
 {
   public:
-    SensorManager(sdbusplus::bus::bus&& pass, sdbusplus::bus::bus&& host) :
-        _passiveListeningBus(std::move(pass)), _hostSensorBus(std::move(host))
+    SensorManager(sdbusplus::bus::bus& pass, sdbusplus::bus::bus& host) :
+        _passiveListeningBus(&pass), _hostSensorBus(&host)
     {
         // manager gets its interface from the bus. :D
-        sdbusplus::server::manager::manager(_hostSensorBus, SensorRoot);
-    }
-
-    SensorManager() :
-        SensorManager(std::move(sdbusplus::bus::new_system()),
-                      std::move(sdbusplus::bus::new_system()))
-    {
+        sdbusplus::server::manager::manager(*_hostSensorBus, SensorRoot);
     }
 
+    SensorManager() = default;
     ~SensorManager() = default;
     SensorManager(const SensorManager&) = delete;
     SensorManager& operator=(const SensorManager&) = delete;
@@ -48,20 +43,20 @@ class SensorManager
 
     sdbusplus::bus::bus& getPassiveBus(void)
     {
-        return _passiveListeningBus;
+        return *_passiveListeningBus;
     }
 
     sdbusplus::bus::bus& getHostBus(void)
     {
-        return _hostSensorBus;
+        return *_hostSensorBus;
     }
 
   private:
     std::map<std::string, std::unique_ptr<Sensor>> _sensorMap;
     std::map<std::string, std::vector<std::string>> _sensorTypeList;
 
-    sdbusplus::bus::bus _passiveListeningBus;
-    sdbusplus::bus::bus _hostSensorBus;
+    sdbusplus::bus::bus* _passiveListeningBus;
+    sdbusplus::bus::bus* _hostSensorBus;
 
     static constexpr auto SensorRoot = "/xyz/openbmc_project/extsensors";
 };
diff --git a/test/pid_zone_unittest.cpp b/test/pid_zone_unittest.cpp
index 2270fa6..11ba4c1 100644
--- a/test/pid_zone_unittest.cpp
+++ b/test/pid_zone_unittest.cpp
@@ -37,7 +37,7 @@ TEST(PidZoneConstructorTest, BoringConstructorTest)
                     IsNull(), _, StrEq("/xyz/openbmc_project/extsensors")))
         .WillOnce(Return(0));
 
-    SensorManager m(std::move(bus_mock_passive), std::move(bus_mock_host));
+    SensorManager m(bus_mock_passive, bus_mock_host);
 
     bool defer = true;
     const char* objPath = "/path/";
@@ -74,7 +74,7 @@ class PidZoneTest : public ::testing::Test
         auto bus_mock_mode = sdbusplus::get_mocked_new(&sdbus_mock_mode);
 
         // Compiler weirdly not happy about just instantiating mgr(...);
-        SensorManager m(std::move(bus_mock_passive), std::move(bus_mock_host));
+        SensorManager m(bus_mock_passive, bus_mock_host);
         mgr = std::move(m);
 
         SetupDbusObject(&sdbus_mock_mode, defer, objPath, modeInterface,
diff --git a/test/sensor_manager_unittest.cpp b/test/sensor_manager_unittest.cpp
index 8a89628..9f0e26c 100644
--- a/test/sensor_manager_unittest.cpp
+++ b/test/sensor_manager_unittest.cpp
@@ -24,7 +24,7 @@ TEST(SensorManagerTest, BoringConstructorTest)
                     IsNull(), _, StrEq("/xyz/openbmc_project/extsensors")))
         .WillOnce(Return(0));
 
-    SensorManager s(std::move(bus_mock_passive), std::move(bus_mock_host));
+    SensorManager s(bus_mock_passive, bus_mock_host);
     // Success
 }
 
@@ -43,7 +43,7 @@ TEST(SensorManagerTest, AddSensorInvalidTypeTest)
                     IsNull(), _, StrEq("/xyz/openbmc_project/extsensors")))
         .WillOnce(Return(0));
 
-    SensorManager s(std::move(bus_mock_passive), std::move(bus_mock_host));
+    SensorManager s(bus_mock_passive, bus_mock_host);
 
     std::string name = "name";
     std::string type = "invalid";
diff --git a/util.hpp b/util.hpp
index 729eaf7..f472bf8 100644
--- a/util.hpp
+++ b/util.hpp
@@ -29,6 +29,8 @@ IOInterfaceType getWriteInterfaceType(const std::string& path);
 
 IOInterfaceType getReadInterfaceType(const std::string& path);
 
+void restartControlLoops(void);
+
 /*
  * Given a configuration structure, fill out the information we use within the
  * PID loop.