monitor: Default tach sensors to true
Instead of reading the functional status of the tach sensors out of the
inventory on startup, just default them to true. Any issues with the
fans could then be rediscovered after the reboot.
This was the original behavior. It was probably changed with the intent
that the shutdown timers could immediately start back up again after a
reboot if things were nonfunctional before.
In practice, we've found that there can be a race between the shutdown
actions turning off the system (due to nonfunctional sensors) and the
sensor objects being marked functional again, even when the only reason
they were nonfunctional before the reboot was because the fan sensor
daemon was turned off before fan monitor on the way down.
For this to make a noticeable change, the shutdown actions/timers would
have to be in progress during the reboot anyway, which is pretty
unlikely.
Worst case, it would extend a shutdown by the time it takes an error to
be rediscovered, which is:
If the 'count' method is configured:
monitor_start_delay + (count_interval * threshold)
If the 'timebased' method is configured:
monitor_start_delay + nonfunc_rotor_error_delay
This has no affect on shutdowns caused by missing fans, as the code
still reads that out of the inventory on startup, plus it can be
instantaneously detected as opposed to being calculated over time.
In summary, extending the shutdown time in very uncommon cases seems
better than mistakenly shutting off a running system, which can be a
huge deal depending on the user.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I2840c5f2e79bd734626b4144713e4428af28551a
diff --git a/monitor/system.cpp b/monitor/system.cpp
index 6e2ccfa..0447cfa 100644
--- a/monitor/system.cpp
+++ b/monitor/system.cpp
@@ -107,6 +107,9 @@
if (_powerState->isPowerOn())
{
+ // Fans could be missing on startup, so check the power off rules.
+ // Tach sensors default to functional, so they wouldn't cause a power
+ // off here.
std::for_each(_powerOffRules.begin(), _powerOffRules.end(),
[this](auto& rule) {
rule->check(PowerRuleState::runtime, _fanHealth);
diff --git a/monitor/tach_sensor.cpp b/monitor/tach_sensor.cpp
index be56c69..660a732 100644
--- a/monitor/tach_sensor.cpp
+++ b/monitor/tach_sensor.cpp
@@ -87,9 +87,6 @@
_timer(event, std::bind(&Fan::updateState, &fan, std::ref(*this))),
_errorDelay(errorDelay), _countInterval(countInterval)
{
- // Query functional state from inventory
- // TODO - phosphor-fan-presence/issues/25
-
_prevTachs.resize(MAX_PREV_TACHS);
if (_hasTarget)
@@ -97,37 +94,8 @@
_prevTargets.resize(MAX_PREV_TARGETS);
}
- _functional = true;
-
- try
- {
- // see if any object paths in the inventory have the
- // OperationalStatus interface.
- auto subtree = util::SDBusPlus::getSubTreeRaw(
- _bus, util::INVENTORY_PATH, util::OPERATIONAL_STATUS_INTF, 0);
-
- // if the tach sensor's entry already exists, we for sure can
- // read its functional state from the inventory
- if (subtree.end() != subtree.find(util::INVENTORY_PATH + _invName))
- {
- _functional = util::SDBusPlus::getProperty<bool>(
- _bus, util::INVENTORY_PATH + _invName,
- util::OPERATIONAL_STATUS_INTF, util::FUNCTIONAL_PROPERTY);
- }
- }
- catch (const util::DBusError& e)
- {
- log<level::DEBUG>(e.what());
- }
-
updateInventory(_functional);
- if (!_functional && MethodMode::count == _method)
- {
- // force continual nonfunctional state
- _counter = _threshold;
- }
-
// Load in current Target and Input values when entering monitor mode
#ifndef MONITOR_USE_JSON
if (mode != Mode::init)