Add more behaviors to MissingIsAcceptable handling
This commit adds more behaviors to the current handling of
MissingIsAcceptable list, under the meson option
`handle-missing-object-paths` which defaults to `false`.
For dbuspassive sensor creation, an input sensor missing from D-Bus or
having D-Bus call failures will be built as a failed sensor with
_failed=true, _available=false, _badReading=true. Therefore, if this
missing sensor is not set as InputUnavailableAsFailed=false and its name
is not listed in the MissingIsAcceptable list, it will be marked as
missing at the beginning, and will send the zone to failsafe mode.
If the sensor comes back to D-Bus, swampd will be informed through the
existing match signals registered.
When parsing configurations from D-Bus, in case of non-fan-type PID
controllers and of Stepwise controllers, if a sensor in the Inputs list
is not on D-Bus and its name is not in the MissingIsAcceptable list, the
sensor will still be built with basic information from the configured
sensor name and type. If it is not on D-Bus and its name is in the
MissingIsAcceptable list, it will be ignored.
When parsing configurations from JSON, no new behavior added in the
building process. As match signals are not registered in this case,
swampd will not be informed with the disappearance and returning of
sensor object paths from/to D-Bus.
Tested:
1. Configure in Entity-Manager a fan controller with a sensor name in
"Inputs". "MissingIsAcceptable" list is not configured.
2. The sensor is not on D-Bus, start swampd
=> The fan zone that the controller belongs to is sent to failsafe mode
3. Get the sensor back to D-Bus (e.g by starting a service)
=> The control loop is restarted due to the match signals from D-Bus, as
this important sensor is back, the fan zone will be back to normal speed
again.
|
| swampd starts parsing configurations
|
+-----------------+-----------------+
| |
| |
v v
+-------+-------+ +----------------+
| DBUS | | JSON |
| configuration | | configuration |
+-------+-------+ +-------+--------+
|
|
v
+--------------------------------------------------------------------+
|Parse "Inputs" for Fan controllers (non-Fan classes only) |
+--------------------------------------------------------------------+
| Loop through "Inputs" of each controller |
| | |
| |-----------------+ |
| |Sensor on D-Bus? |------ Yes----> construct configs-----+ |
| +-----------------+ with DBus info | |
| | No | |
| v | |
| |-------------------+ | |
| |MissingAcceptable? |------No----> construct configs-----+ |
| +-------------------+ with basic info | |
| | Yes | |
| v v |
| continue continue |
+--------------------------------------------------------------------+
|
|
v
+--------------------------------------------------------------------+
|Create dbuspassive sensor |
+--------------------------------------------------------------------+
| |---------------------+ |
| | DBus calls succeed? |------ Yes----------------+ |
| +---------------------+ | |
| | No | |
| v v |
|construct properties with construct properties |
|with bad info with DBus info |
|_value{nan} |
|_unit{based_on_type} |
|_scale{0} |
|_available{false} |
|_failed{true} |
| |
+--------------------------------------------------------------------+
Change-Id: I926136d3d81f19d4c6d2e5b377cdedb887e49e5f
Signed-off-by: Chaul Ly <chaul@amperecomputing.com>
diff --git a/dbus/dbusconfiguration.cpp b/dbus/dbusconfiguration.cpp
index b1ff0b7..9de25ad 100644
--- a/dbus/dbusconfiguration.cpp
+++ b/dbus/dbusconfiguration.cpp
@@ -13,6 +13,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
*/
+#include "config.h"
+
#include "dbusconfiguration.hpp"
#include "conf.hpp"
@@ -686,6 +688,7 @@
std::get<std::vector<std::string>>(base.at("Inputs")));
std::vector<std::string> outputSensorNames;
std::vector<std::string> missingAcceptableSensorNames;
+ std::vector<std::string> archivedInputSensorNames;
auto findMissingAcceptable = base.find("MissingIsAcceptable");
if (findMissingAcceptable != base.end())
@@ -726,8 +729,55 @@
*/
for (const std::string& sensorName : inputSensorNames)
{
+#ifndef HANDLE_MISSING_OBJECT_PATHS
findSensors(sensors, sensorNameToDbusName(sensorName),
inputSensorInterfaces);
+#else
+ std::vector<std::pair<std::string, std::string>>
+ sensorPathIfacePairs;
+ auto found =
+ findSensors(sensors, sensorNameToDbusName(sensorName),
+ sensorPathIfacePairs);
+ if (found)
+ {
+ inputSensorInterfaces.insert(
+ inputSensorInterfaces.end(),
+ sensorPathIfacePairs.begin(),
+ sensorPathIfacePairs.end());
+ }
+ else if (pidClass != "fan")
+ {
+ if (std::find(missingAcceptableSensorNames.begin(),
+ missingAcceptableSensorNames.end(),
+ sensorName) ==
+ missingAcceptableSensorNames.end())
+ {
+ std::cerr
+ << "Pid controller: Missing a missing-unacceptable sensor from D-Bus "
+ << sensorName << "\n";
+ std::string inputSensorName =
+ sensorNameToDbusName(sensorName);
+ auto& config = sensorConfig[inputSensorName];
+ archivedInputSensorNames.push_back(inputSensorName);
+ config.type = pidClass;
+ config.readPath =
+ getSensorPath(config.type, inputSensorName);
+ config.timeout = 0;
+ config.ignoreDbusMinMax = true;
+ config.unavailableAsFailed = unavailableAsFailed;
+ }
+ else
+ {
+ // When an input sensor is NOT on DBus, and it's in
+ // the MissingIsAcceptable list. Ignore it and
+ // continue with the next input sensor.
+ std::cout
+ << "Pid controller: Missing a missing-acceptable sensor from D-Bus "
+ << sensorName << "\n";
+ continue;
+ }
+ }
+#endif
}
for (const std::string& sensorName : outputSensorNames)
{
@@ -741,7 +791,6 @@
missingAcceptableSensorInterfaces);
}
- inputSensorNames.clear();
for (const SensorInterfaceType& inputSensorInterface :
inputSensorInterfaces)
{
@@ -764,7 +813,7 @@
std::string inputSensorName =
getSensorNameFromPath(inputSensorPath);
auto& config = sensorConfig[inputSensorName];
- inputSensorNames.push_back(inputSensorName);
+ archivedInputSensorNames.push_back(inputSensorName);
config.type = pidClass;
config.readPath = inputSensorInterface.first;
config.timeout = 0;
@@ -879,7 +928,7 @@
fanSensorName = getSensorNameFromPath(fanPath);
pwmSensorName = getSensorNameFromPath(pwmPath);
std::string fanPwmIndex = fanSensorName + pwmSensorName;
- inputSensorNames.push_back(fanPwmIndex);
+ archivedInputSensorNames.push_back(fanPwmIndex);
auto& fanConfig = sensorConfig[fanPwmIndex];
fanConfig.type = pidClass;
fanConfig.readPath = fanPath;
@@ -892,7 +941,7 @@
}
// if the sensors aren't available in the current state, don't
// add them to the configuration.
- if (inputSensorNames.empty())
+ if (archivedInputSensorNames.empty())
{
continue;
}
@@ -926,7 +975,7 @@
}
std::vector<pid_control::conf::SensorInput> sensorInputs =
- spliceInputs(inputSensorNames, inputTempToMargin,
+ spliceInputs(archivedInputSensorNames, inputTempToMargin,
missingAcceptableSensorNames);
if (offsetType.empty())
@@ -996,25 +1045,69 @@
if (!findSensors(sensors, sensorNameToDbusName(sensorName),
sensorPathIfacePairs))
{
+#ifndef HANDLE_MISSING_OBJECT_PATHS
break;
+#else
+ if (std::find(missingAcceptableSensorNames.begin(),
+ missingAcceptableSensorNames.end(),
+ sensorName) ==
+ missingAcceptableSensorNames.end())
+ {
+ // When an input sensor is NOT on DBus, and it's NOT
+ // in the MissingIsAcceptable list. Build it as a
+ // failed sensor with default information (temp
+ // sensor path, temp type, ...)
+ std::cerr
+ << "Stepwise controller: Missing a missing-unacceptable sensor from D-Bus "
+ << sensorName << "\n";
+ std::string shortName =
+ sensorNameToDbusName(sensorName);
+
+ inputs.push_back(shortName);
+ auto& config = sensorConfig[shortName];
+ config.type = "temp";
+ config.readPath =
+ getSensorPath(config.type, shortName);
+ config.ignoreDbusMinMax = true;
+ config.unavailableAsFailed = unavailableAsFailed;
+ // todo: maybe un-hardcode this if we run into
+ // slower timeouts with sensors
+
+ config.timeout = 0;
+ sensorFound = true;
+ }
+ else
+ {
+ // When an input sensor is NOT on DBus, and it's in
+ // the MissingIsAcceptable list. Ignore it and
+ // continue with the next input sensor.
+ std::cout
+ << "Stepwise controller: Missing a missing-acceptable sensor from D-Bus "
+ << sensorName << "\n";
+ continue;
+ }
+#endif
}
-
- for (const auto& sensorPathIfacePair : sensorPathIfacePairs)
+ else
{
- std::string shortName =
- getSensorNameFromPath(sensorPathIfacePair.first);
+ for (const auto& sensorPathIfacePair :
+ sensorPathIfacePairs)
+ {
+ std::string shortName = getSensorNameFromPath(
+ sensorPathIfacePair.first);
- inputs.push_back(shortName);
- auto& config = sensorConfig[shortName];
- config.readPath = sensorPathIfacePair.first;
- config.type = "temp";
- config.ignoreDbusMinMax = true;
- config.unavailableAsFailed = unavailableAsFailed;
- // todo: maybe un-hardcode this if we run into slower
- // timeouts with sensors
+ inputs.push_back(shortName);
+ auto& config = sensorConfig[shortName];
+ config.readPath = sensorPathIfacePair.first;
+ config.type = "temp";
+ config.ignoreDbusMinMax = true;
+ config.unavailableAsFailed = unavailableAsFailed;
+ // todo: maybe un-hardcode this if we run into
+ // slower timeouts with sensors
- config.timeout = 0;
- sensorFound = true;
+ config.timeout = 0;
+ sensorFound = true;
+ }
}
}
if (!sensorFound)
@@ -1033,7 +1126,15 @@
sensorNameToDbusName(missingAcceptableSensorName),
sensorPathIfacePairs))
{
+#ifndef HANDLE_MISSING_OBJECT_PATHS
break;
+#else
+ // When a sensor in the MissingIsAcceptable list is NOT
+ // on DBus and it still reaches here, which contradicts
+ // to what we did in the Input sensor building step.
+ // Continue.
+ continue;
+#endif
}
for (const auto& sensorPathIfacePair : sensorPathIfacePairs)
diff --git a/dbus/dbuspassive.cpp b/dbus/dbuspassive.cpp
index 160dbde..1d88b60 100644
--- a/dbus/dbuspassive.cpp
+++ b/dbus/dbuspassive.cpp
@@ -76,7 +76,39 @@
}
catch (const std::exception& e)
{
+#ifndef HANDLE_MISSING_OBJECT_PATHS
return nullptr;
+#else
+ // CASE1: The sensor is not on DBus, but as it is not in the
+ // MissingIsAcceptable list, the sensor should be built with a failed
+ // state to send the zone to failsafe mode. Everything will recover if
+ // all important sensors are back to DBus. swampd will be informed
+ // through InterfacesAdded signals and the sensors will be built again.
+
+ // CASE2: The sensor is in the MissingIsAcceptable list and it EXISTS on
+ // DBus (which sends it all the way here). However, swampd fails to
+ // initialize its setting here because of some DBus error???
+ // (getService/getProperties/getThresholdAssertion). Build it as a
+ // failed sensor too. A DBus signal will inform if there's s new
+ // property value to the sensor and will recover its state when the new
+ // value is valid.
+
+ // In both cases, the Sensor::getFailed() and
+ // DbusPidZone::markSensorMissing() APIs will decide whether to add a
+ // failed sensor to the _failSafeSensors list. As _failed=true,
+ // _available=false and _badReading=false (due to updateValue(nan,
+ // true)), both cases will have getFailed()=true at the beginning as
+ // long as _unavailableAsFailed=true; However as CASE2 has the sensor in
+ // MissingIsAcceptable list, only CASE1 will send the zone to failSafe
+ // mode.
+
+ failed = true;
+ settings.value = std::numeric_limits<double>::quiet_NaN();
+ settings.unit = getSensorUnit(type);
+ settings.available = false;
+ std::cerr << "DbusPassive: Sensor " << path
+ << " is missing from D-Bus, build this sensor as failed\n";
+#endif
}
/* if these values are zero, they're ignored. */
diff --git a/dbus/dbusutil.cpp b/dbus/dbusutil.cpp
index 7d3704d..18d8f36 100644
--- a/dbus/dbusutil.cpp
+++ b/dbus/dbusutil.cpp
@@ -90,6 +90,28 @@
return matches.size() > 0;
}
+std::string getSensorUnit(const std::string& type)
+{
+ std::string unit;
+ if (type == "fan")
+ {
+ unit = "xyz.openbmc_project.Sensor.Value.Unit.RPMS";
+ }
+ else if (type == "temp" || type == "margin")
+ {
+ unit = "xyz.openbmc_project.Sensor.Value.Unit.DegreesC";
+ }
+ else if (type == "power" || type == "powersum")
+ {
+ unit = "xyz.openbmc_project.Sensor.Value.Unit.Watts";
+ }
+ else
+ {
+ unit = "unknown";
+ }
+ return unit;
+}
+
std::string getSensorPath(const std::string& type, const std::string& id)
{
std::string layer;
diff --git a/dbus/dbusutil.hpp b/dbus/dbusutil.hpp
index ff606a4..ee8d166 100644
--- a/dbus/dbusutil.hpp
+++ b/dbus/dbusutil.hpp
@@ -28,6 +28,7 @@
}
};
+std::string getSensorUnit(const std::string& type);
std::string getSensorPath(const std::string& type, const std::string& id);
std::string getMatch(const std::string& path);
void scaleSensorReading(const double min, const double max, double& value);
diff --git a/meson.build b/meson.build
index 21af915..06c9404 100644
--- a/meson.build
+++ b/meson.build
@@ -21,6 +21,10 @@
conf_data.set('STRICT_FAILSAFE_PWM', get_option('strict-failsafe-pwm'))
conf_data.set('OFFLINE_FAILSAFE_PWM', get_option('offline-failsafe-pwm'))
conf_data.set('UNC_FAILSAFE', get_option('unc-failsafe'))
+conf_data.set(
+ 'HANDLE_MISSING_OBJECT_PATHS',
+ get_option('handle-missing-object-paths'),
+)
configure_file(output: 'config.h', configuration: conf_data)
diff --git a/meson.options b/meson.options
index f4182ca..a2d1e3c 100644
--- a/meson.options
+++ b/meson.options
@@ -29,3 +29,9 @@
value: false,
description: 'Set the fans to failsafe mode when a temp sensor is above upper non-critical threshold',
)
+option(
+ 'handle-missing-object-paths',
+ type: 'boolean',
+ value: false,
+ description: 'Further handlings to sensors missing from D-Bus',
+)