Fix PSU threshold mismatch issue
When trying to change thresholds on PSU sensors, PSU service will crash.
Some PSU in entity-manager will have threshold0 to threshodl5 which
contains all labels in one D-Bus path. But in current dbus sensors,
the presist threshold function do not support labels, it only check
severity. So when change a threshold in PSU sensor, it will change some
incorrect threshold in entity-manger. This patch adds label support to
presist threshold function, so that correct theshold in entity-manager
can be changed.
Tested:
After changed PSU1 Temp1 threshold by ipmitool, check the threshold again,
the value should be changed to the value we set. Check entity-manager,
the threshold in entity-manager should also change to the value we set.
Signed-off-by: Cheng C Yang <cheng.c.yang@linux.intel.com>
Change-Id: Ib1c8bb454cd42dff170ae33c4aa768c4b515bb44
diff --git a/include/PSUSensor.hpp b/include/PSUSensor.hpp
index 14b6d96..98ec3d7 100644
--- a/include/PSUSensor.hpp
+++ b/include/PSUSensor.hpp
@@ -18,7 +18,7 @@
std::vector<thresholds::Threshold>&& thresholds,
const std::string& sensorConfiguration,
std::string& sensorTypeName, unsigned int factor, double max,
- double min);
+ double min, const std::string& label, size_t tSize);
~PSUSensor();
private:
diff --git a/include/Thresholds.hpp b/include/Thresholds.hpp
index 00df68c..20ce69e 100644
--- a/include/Thresholds.hpp
+++ b/include/Thresholds.hpp
@@ -118,10 +118,11 @@
void persistThreshold(const std::string& baseInterface, const std::string& path,
const thresholds::Threshold& threshold,
std::shared_ptr<sdbusplus::asio::connection>& conn,
- size_t thresholdCount);
+ size_t thresholdCount, const std::string& label);
void updateThresholds(Sensor* sensor);
// returns false if a critical threshold has been crossed, true otherwise
bool checkThresholds(Sensor* sensor);
void checkThresholdsPowerDelay(Sensor* sensor, ThresholdTimer& thresholdTimer);
+
} // namespace thresholds
diff --git a/include/sensor.hpp b/include/sensor.hpp
index e298677..3c4706f 100644
--- a/include/sensor.hpp
+++ b/include/sensor.hpp
@@ -60,7 +60,9 @@
}
void
- setInitialProperties(std::shared_ptr<sdbusplus::asio::connection>& conn)
+ setInitialProperties(std::shared_ptr<sdbusplus::asio::connection>& conn,
+ const std::string label = std::string(),
+ size_t thresholdSize = 0)
{
createAssociation(association, configurationPath);
@@ -114,14 +116,17 @@
std::cout << "trying to set uninitialized interface\n";
continue;
}
+
+ size_t thresSize =
+ label.empty() ? thresholds.size() : thresholdSize;
iface->register_property(
level, threshold.value,
- [&](const double& request, double& oldValue) {
+ [&, label, thresSize](const double& request, double& oldValue) {
oldValue = request; // todo, just let the config do this?
threshold.value = request;
thresholds::persistThreshold(configurationPath, objectType,
- threshold, conn,
- thresholds.size());
+ threshold, conn, thresSize,
+ label);
// Invalidate previously remembered value,
// so new thresholds will be checked during next update,
// even if sensor reading remains unchanged.
diff --git a/src/PSUSensor.cpp b/src/PSUSensor.cpp
index 96b710c..2996b6f 100644
--- a/src/PSUSensor.cpp
+++ b/src/PSUSensor.cpp
@@ -41,13 +41,12 @@
std::vector<thresholds::Threshold>&& _thresholds,
const std::string& sensorConfiguration,
std::string& sensorTypeName, unsigned int factor,
- double max, double min) :
+ double max, double min, const std::string& label,
+ size_t tSize) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"),
std::move(_thresholds), sensorConfiguration, objectType, max, min),
objServer(objectServer), inputDev(io), waitTimer(io), path(path),
- errCount(0),
-
- sensorFactor(factor)
+ errCount(0), sensorFactor(factor)
{
if constexpr (DEBUG)
{
@@ -85,7 +84,14 @@
// This should be called before initializing association.
// createInventoryAssoc() does add more associations before doing
// register and initialize "Associations" property.
- setInitialProperties(conn);
+ if (label.empty() || tSize == _thresholds.size())
+ {
+ setInitialProperties(conn);
+ }
+ else
+ {
+ setInitialProperties(conn, label, tSize);
+ }
association = objectServer.add_interface(dbusPath, association::interface);
@@ -97,6 +103,7 @@
{
waitTimer.cancel();
inputDev.close();
+ objServer.remove_interface(association);
objServer.remove_interface(sensorInterface);
objServer.remove_interface(thresholdInterfaceWarning);
objServer.remove_interface(thresholdInterfaceCritical);
@@ -115,7 +122,7 @@
{
if (err == boost::system::errc::bad_file_descriptor)
{
- std::cerr << "Bad file descriptor from " << path << "\n";
+ std::cerr << "Bad file descriptor from\n";
return;
}
std::istream responseStream(&readBuf);
@@ -134,14 +141,13 @@
}
catch (const std::invalid_argument&)
{
- std::cerr << "Could not parse " << response << " from path " << path
- << "\n";
+ std::cerr << "Could not parse " << response << "\n";
errCount++;
}
}
else
{
- std::cerr << "System error " << err << " from path " << path << "\n";
+ std::cerr << "System error " << err << "\n";
errCount++;
}
@@ -149,8 +155,7 @@
{
if (errCount == warnAfterErrorCount)
{
- std::cerr << "Failure to read sensor " << name << " at " << path
- << "\n";
+ std::cerr << "Failure to read sensor " << name << "\n";
}
updateValue(0);
errCount++;
@@ -161,7 +166,7 @@
waitTimer.async_wait([&](const boost::system::error_code& ec) {
if (ec == boost::asio::error::operation_aborted)
{
- std::cerr << "Failed to reschedule wait for " << path << "\n";
+ std::cerr << "Failed to reschedule\n";
return;
}
setupRead();
diff --git a/src/PSUSensorMain.cpp b/src/PSUSensorMain.cpp
index 0367d2a..64f54da 100644
--- a/src/PSUSensorMain.cpp
+++ b/src/PSUSensorMain.cpp
@@ -218,7 +218,6 @@
// TODO may need only modify the ones that need to be changed.
sensors.clear();
- combineEvents.clear();
for (const char* type : sensorTypes)
{
if (!getSensorConfiguration(type, dbusConnection, sensorConfigs,
@@ -310,6 +309,7 @@
const SensorData* sensorData = nullptr;
const std::string* interfacePath = nullptr;
const char* sensorType = nullptr;
+ size_t thresholdConfSize = 0;
for (const std::pair<sdbusplus::message::object_path, SensorData>&
sensor : sensorConfigs)
@@ -360,6 +360,13 @@
continue;
}
+ std::vector<thresholds::Threshold> confThresholds;
+ if (!parseThresholdsFromConfig(*sensorData, confThresholds))
+ {
+ std::cerr << "error populating totoal thresholds\n";
+ }
+ thresholdConfSize = confThresholds.size();
+
interfacePath = &(sensor.first.str);
break;
}
@@ -441,9 +448,11 @@
std::ifstream labelFile(labelPath);
if (!labelFile.good())
{
- std::cerr << "Input file " << sensorPath
- << " has no corresponding label file\n";
-
+ if constexpr (DEBUG)
+ {
+ std::cerr << "Input file " << sensorPath
+ << " has no corresponding label file\n";
+ }
// hwmon *_input filename with number:
// temp1, temp2, temp3, ...
labelHead = sensorNameStr.substr(0, sensorNameStr.find("_"));
@@ -479,8 +488,11 @@
if (std::find(findLabels.begin(), findLabels.end(),
labelHead) == findLabels.end())
{
- std::cerr << "could not find " << labelHead
- << " in the Labels list\n";
+ if constexpr (DEBUG)
+ {
+ std::cerr << "could not find " << labelHead
+ << " in the Labels list\n";
+ }
continue;
}
}
@@ -669,7 +681,6 @@
}
std::vector<thresholds::Threshold> sensorThresholds;
-
if (!parseThresholdsFromConfig(*sensorData, sensorThresholds,
&labelHead))
{
@@ -724,7 +735,7 @@
sensorPathStr, sensorType, objectServer, dbusConnection, io,
sensorName, std::move(sensorThresholds), *interfacePath,
findSensorType->second, factor, psuProperty->maxReading,
- psuProperty->minReading);
+ psuProperty->minReading, labelHead, thresholdConfSize);
++numCreated;
if constexpr (DEBUG)
diff --git a/src/Thresholds.cpp b/src/Thresholds.cpp
index 0b33829..b5e8685 100644
--- a/src/Thresholds.cpp
+++ b/src/Thresholds.cpp
@@ -118,14 +118,14 @@
void persistThreshold(const std::string& path, const std::string& baseInterface,
const thresholds::Threshold& threshold,
std::shared_ptr<sdbusplus::asio::connection>& conn,
- size_t thresholdCount)
+ size_t thresholdCount, const std::string& labelMatch)
{
for (size_t ii = 0; ii < thresholdCount; ii++)
{
std::string thresholdInterface =
baseInterface + ".Thresholds" + std::to_string(ii);
conn->async_method_call(
- [&, path, threshold, thresholdInterface](
+ [&, path, threshold, thresholdInterface, labelMatch](
const boost::system::error_code& ec,
const boost::container::flat_map<std::string, BasicVariantType>&
result) {
@@ -134,6 +134,22 @@
return; // threshold not supported
}
+ if (!labelMatch.empty())
+ {
+ auto labelFind = result.find("Label");
+ if (labelFind == result.end())
+ {
+ std::cerr << "No label in threshold configuration\n";
+ return;
+ }
+ std::string label =
+ std::visit(VariantToStringVisitor(), labelFind->second);
+ if (label != labelMatch)
+ {
+ return;
+ }
+ }
+
auto directionFind = result.find("Direction");
auto severityFind = result.find("Severity");
auto valueFind = result.find("Value");