commit 9ff48777feba12a836b209b0913335c64c30c8bf
author Mike Capps <mikepcapps@gmail.com> Mon Jul 19 14:49:43 2021 -0400
committer Mike Capps <mikepcapps@gmail.com> Tue Jul 20 09:32:46 2021 -0400
tree cb5e276424f712cdb317c403af639febc7167c7d
parent 9d76388d9251adfc500d8456687e869322341033

monitor : do not trigger error when inventory unavailable

Under some startup scenarios, fan-monitor can start before the
InventoryManager service has populated inventory. This leads to
false-positives when marking fans non-functional.

This fix detects D-Bus exceptions and defaults the fans to functional.
They subscribe to the interfaces-added signal which will populate the
fans with correct values as soon as Inventory Manager completes startup.

Signed-off-by: Mike Capps <mikepcapps@gmail.com>
Change-Id: I14bff20da14cba3f5ef4b79763867b5cecab0267

monitor/fan.cpp

diff --git a/monitor/fan.cpp b/monitor/fan.cpp
index 5f0414d..c5322b1 100644
--- a/monitor/fan.cpp
+++ b/monitor/fan.cpp
@@ -84,7 +84,16 @@
         (_numSensorFailsForNonFunc == 0) ||
         (countNonFunctionalSensors() < _numSensorFailsForNonFunc);
 
-    updateInventory(functionalState);
+    if (updateInventory(functionalState) && !functionalState)
+    {
+        // the inventory update threw an exception, possibly because D-Bus
+        // wasn't ready. Try to update sensors back to functional to avoid a
+        // false-alarm. They will be updated again from subscribing to the
+        // properties-changed event
+
+        for (auto& sensor : _sensors)
+            sensor->setFunctional(true);
+    }
 
 #ifndef MONITOR_USE_JSON
     // Check current tach state when entering monitor mode
@@ -406,21 +415,41 @@
     _system.fanStatusChange(*this);
 }
 
-void Fan::updateInventory(bool functional)
+bool Fan::updateInventory(bool functional)
 {
-    auto objectMap =
-        util::getObjMap<bool>(_name, util::OPERATIONAL_STATUS_INTF,
-                              util::FUNCTIONAL_PROPERTY, functional);
-    auto response = util::SDBusPlus::lookupAndCallMethod(
-        _bus, util::INVENTORY_PATH, util::INVENTORY_INTF, "Notify", objectMap);
-    if (response.is_method_error())
+    bool dbusError = false;
+
+    try
     {
-        log<level::ERR>("Error in Notify call to update inventory");
-        return;
+        auto objectMap =
+            util::getObjMap<bool>(_name, util::OPERATIONAL_STATUS_INTF,
+                                  util::FUNCTIONAL_PROPERTY, functional);
+
+        auto response = util::SDBusPlus::lookupAndCallMethod(
+            _bus, util::INVENTORY_PATH, util::INVENTORY_INTF, "Notify",
+            objectMap);
+
+        if (response.is_method_error())
+        {
+            log<level::ERR>("Error in Notify call to update inventory");
+
+            dbusError = true;
+        }
+    }
+    catch (const util::DBusError& e)
+    {
+        dbusError = true;
+
+        getLogger().log(
+            fmt::format("D-Bus Exception reading/updating inventory : {}",
+                        e.what()),
+            Logger::error);
     }
 
     // This will always track the current state of the inventory.
     _functional = functional;
+
+    return dbusError;
 }
 
 void Fan::presenceChanged(sdbusplus::message::message& msg)

monitor/fan.hpp

diff --git a/monitor/fan.hpp b/monitor/fan.hpp
index d4bcb2d..8d9f3b2 100644
--- a/monitor/fan.hpp
+++ b/monitor/fan.hpp
@@ -213,8 +213,10 @@
      *
      * @param[in] functional - If the Functional property should
      *                         be set to true or false.
+     *
+     * @return - True if an exception was encountered during update
      */
-    void updateInventory(bool functional);
+    bool updateInventory(bool functional);
 
     /**
      * @brief Called by _monitorTimer to start fan monitoring some

monitor/tach_sensor.cpp

diff --git a/monitor/tach_sensor.cpp b/monitor/tach_sensor.cpp
index cc35c97..daeb1f0 100644
--- a/monitor/tach_sensor.cpp
+++ b/monitor/tach_sensor.cpp
@@ -84,11 +84,13 @@
 {
     // Query functional state from inventory
     // TODO - phosphor-fan-presence/issues/25
-    auto service = util::SDBusPlus::getService(
-        _bus, util::INVENTORY_PATH + _invName, util::OPERATIONAL_STATUS_INTF);
 
     try
     {
+        auto service =
+            util::SDBusPlus::getService(_bus, util::INVENTORY_PATH + _invName,
+                                        util::OPERATIONAL_STATUS_INTF);
+
         if (!service.empty())
         {
             _functional = util::SDBusPlus::getProperty<bool>(
@@ -100,14 +102,12 @@
             // default to functional when service not up. Error handling done
             // later
             _functional = true;
-            updateInventory(_functional);
         }
     }
     catch (util::DBusError& e)
     {
         log<level::DEBUG>(e.what());
         _functional = true;
-        updateInventory(_functional);
     }
 
     if (!_functional && MethodMode::count == _method)