sensor-cache: Invalidate cache when sensor disappears
When a sensor disappears from DBus, invalidate the cache.
Ideally it could be done by adding a match for interfacesRemoved
signal.
However, interfacesRemoved signal is not emitted if a service is
terminated or crashed, so we have to use nameOwnerChanged signal
as well. The `nameOwnerChanged` signal does not provide DBus object's
information, so the code needs to record the relationship between sensor
ids and the services, so that when the event occurs, it knows which
sensors should be invalidated.
Tested: Manually stop virtual-sensor, and verify the related sensors are
shown as `na` in ipmitool sensor list:
total_power | na | | na | na | na | na | na | na | na
Restart virtual-sensor, and verify the related sensors have
valid readings:
total_power | 510.000 | Watts | ok | na | na | na | na | na | na
Signed-off-by: Lei YU <yulei.sh@bytedance.com>
Change-Id: If145a1cd163477c3aca3fd17f3dbde96263f6b75
diff --git a/sensorhandler.cpp b/sensorhandler.cpp
index 6cc34f9..5464d0e 100644
--- a/sensorhandler.cpp
+++ b/sensorhandler.cpp
@@ -91,9 +91,56 @@
sensorAddedMatches __attribute__((init_priority(101)));
std::map<uint8_t, std::unique_ptr<sdbusplus::bus::match::match>>
sensorUpdatedMatches __attribute__((init_priority(101)));
+std::map<uint8_t, std::unique_ptr<sdbusplus::bus::match::match>>
+ sensorRemovedMatches __attribute__((init_priority(101)));
+std::unique_ptr<sdbusplus::bus::match::match> sensorsOwnerMatch
+ __attribute__((init_priority(101)));
ipmi::sensor::SensorCacheMap sensorCacheMap __attribute__((init_priority(101)));
+// It is needed to know which objects belong to which service, so that when a
+// service exits without interfacesRemoved signal, we could invaildate the cache
+// that is related to the service. It uses below two variables:
+// - idToServiceMap records which sensors are known to have a related service;
+// - serviceToIdMap maps a service to the sensors.
+using sensorIdToServiceMap = std::unordered_map<uint8_t, std::string>;
+sensorIdToServiceMap idToServiceMap __attribute__((init_priority(101)));
+
+using sensorServiceToIdMap = std::unordered_map<std::string, std::set<uint8_t>>;
+sensorServiceToIdMap serviceToIdMap __attribute__((init_priority(101)));
+
+static void fillSensorIdServiceMap(const std::string& obj,
+ const std::string& /*intf*/, uint8_t id,
+ const std::string& service)
+{
+ if (idToServiceMap.find(id) != idToServiceMap.end())
+ {
+ return;
+ }
+ idToServiceMap[id] = service;
+ serviceToIdMap[service].insert(id);
+}
+
+static void fillSensorIdServiceMap(const std::string& obj,
+ const std::string& intf, uint8_t id)
+{
+ if (idToServiceMap.find(id) != idToServiceMap.end())
+ {
+ return;
+ }
+ try
+ {
+ sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
+ auto service = ipmi::getService(bus, intf, obj);
+ idToServiceMap[id] = service;
+ serviceToIdMap[service].insert(id);
+ }
+ catch (...)
+ {
+ // Ignore
+ }
+}
+
void initSensorMatches()
{
using namespace sdbusplus::bus::match::rules;
@@ -104,8 +151,19 @@
s.first,
std::make_unique<sdbusplus::bus::match::match>(
bus, interfacesAdded() + argNpath(0, s.second.sensorPath),
- [id = s.first, obj = s.second.sensorPath](auto& /*msg*/) {
- // TODO
+ [id = s.first, obj = s.second.sensorPath,
+ intf = s.second.propertyInterfaces.begin()->first](
+ auto& /*msg*/) { fillSensorIdServiceMap(obj, intf, id); }));
+ sensorRemovedMatches.emplace(
+ s.first,
+ std::make_unique<sdbusplus::bus::match::match>(
+ bus, interfacesRemoved() + argNpath(0, s.second.sensorPath),
+ [id = s.first](auto& /*msg*/) {
+ // Ideally this should work.
+ // But when a service is terminated or crashed, it does not
+ // emit interfacesRemoved signal. In that case it's handled
+ // by sensorsOwnerMatch
+ sensorCacheMap[id].reset();
}));
sensorUpdatedMatches.emplace(
s.first, std::make_unique<sdbusplus::bus::match::match>(
@@ -114,6 +172,10 @@
member("PropertiesChanged"s) +
interface("org.freedesktop.DBus.Properties"s),
[&s](auto& msg) {
+ fillSensorIdServiceMap(
+ s.second.sensorPath,
+ s.second.propertyInterfaces.begin()->first,
+ s.first);
try
{
// This is signal callback
@@ -128,6 +190,28 @@
sensorCacheMap[s.first].reset();
}
}));
+ sensorsOwnerMatch = std::make_unique<sdbusplus::bus::match::match>(
+ bus, nameOwnerChanged(), [](auto& msg) {
+ std::string name;
+ std::string oldOwner;
+ std::string newOwner;
+ msg.read(name, oldOwner, newOwner);
+
+ if (!name.empty() && newOwner.empty())
+ {
+ // The service exits
+ const auto it = serviceToIdMap.find(name);
+ if (it == serviceToIdMap.end())
+ {
+ return;
+ }
+ for (const auto& id : it->second)
+ {
+ // Invalidate cache
+ sensorCacheMap[id].reset();
+ }
+ }
+ });
}
}
#endif
@@ -510,6 +594,9 @@
{
return ipmi::responseUnspecifiedError();
}
+ fillSensorIdServiceMap(sensorInfo.sensorPath,
+ sensorInfo.propertyInterfaces.begin()->first,
+ iter->first, service);
ipmi::PropertyMap props;
ec = ipmi::getAllDbusProperties(