Handle losing association endpoints
If the endpoint path of an association (not the path that has the
org.openbmc.Associations interface) goes off of D-Bus, then remove the 2
association objects and move this association to the list of pending
ones. That way if it ever comes back, the association objects will be
re-added.
This commit adds a moveAssociationToPending function in the code paths
where the mapper sees D-Bus paths going away. That function will find
all associations that involve that path, and then remove the actual
association paths and add them to the list of pending ones.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I14d5ddf8f65be866c2cedd5f467d65adf8e3af95
diff --git a/src/associations.cpp b/src/associations.cpp
index 1393ed9..110c6eb 100644
--- a/src/associations.cpp
+++ b/src/associations.cpp
@@ -469,3 +469,130 @@
}
}
}
+
+/** @brief Remove an endpoint for a particular association from D-Bus.
+ *
+ * If the last endpoint is gone, remove the whole association interface,
+ * otherwise just update the D-Bus endpoints property.
+ *
+ * @param[in] assocPath - the association path
+ * @param[in] endpointPath - the endpoint path to find and remove
+ * @param[in,out] assocMaps - the association maps
+ * @param[in,out] server - sdbus system object
+ */
+void removeAssociationIfacesEntry(const std::string& assocPath,
+ const std::string& endpointPath,
+ AssociationMaps& assocMaps,
+ sdbusplus::asio::object_server& server)
+{
+ auto assoc = assocMaps.ifaces.find(assocPath);
+ if (assoc != assocMaps.ifaces.end())
+ {
+ auto& endpoints = std::get<endpointsPos>(assoc->second);
+ auto e = std::find(endpoints.begin(), endpoints.end(), endpointPath);
+ if (e != endpoints.end())
+ {
+ endpoints.erase(e);
+
+ if (endpoints.empty())
+ {
+ server.remove_interface(std::get<ifacePos>(assoc->second));
+ std::get<ifacePos>(assoc->second) = nullptr;
+ }
+ else
+ {
+ std::get<ifacePos>(assoc->second)
+ ->set_property("endpoints", endpoints);
+ }
+ }
+ }
+}
+
+/** @brief Remove an endpoint from the association owners map.
+ *
+ * For a specific association path and owner, remove the endpoint.
+ * Remove all remaining artifacts of that endpoint in the owners map
+ * based on what frees up after the erase.
+ *
+ * @param[in] assocPath - the association object path
+ * @param[in] endpointPath - the endpoint object path
+ * @param[in] owner - the owner of the association
+ * @param[in,out] assocMaps - the association maps
+ * @param[in,out] server - sdbus system object
+ */
+void removeAssociationOwnersEntry(const std::string& assocPath,
+ const std::string& endpointPath,
+ const std::string& owner,
+ AssociationMaps& assocMaps,
+ sdbusplus::asio::object_server& server)
+{
+ auto sources = assocMaps.owners.begin();
+ while (sources != assocMaps.owners.end())
+ {
+ auto owners = sources->second.find(owner);
+ if (owners != sources->second.end())
+ {
+ auto entry = owners->second.find(assocPath);
+ if (entry != owners->second.end())
+ {
+ auto e = std::find(entry->second.begin(), entry->second.end(),
+ endpointPath);
+ if (e != entry->second.end())
+ {
+ entry->second.erase(e);
+ if (entry->second.empty())
+ {
+ owners->second.erase(entry);
+ }
+ }
+ }
+
+ if (owners->second.empty())
+ {
+ sources->second.erase(owners);
+ }
+ }
+
+ if (sources->second.empty())
+ {
+ sources = assocMaps.owners.erase(sources);
+ continue;
+ }
+ sources++;
+ }
+}
+
+void moveAssociationToPending(const std::string& endpointPath,
+ AssociationMaps& assocMaps,
+ sdbusplus::asio::object_server& server)
+{
+ FindAssocResults associationData;
+
+ // Check which associations this path is an endpoint of, and
+ // then add them to the pending associations map and remove
+ // the associations objects.
+ findAssociations(endpointPath, assocMaps, associationData);
+
+ for (const auto& [owner, association] : associationData)
+ {
+ const auto& forwardPath = endpointPath;
+ const auto& forwardType = std::get<forwardTypePos>(association);
+ const auto& reversePath = std::get<reversePathPos>(association);
+ const auto& reverseType = std::get<reverseTypePos>(association);
+
+ addPendingAssociation(forwardPath, forwardType, reversePath,
+ reverseType, owner, assocMaps);
+
+ // Remove both sides of the association from assocMaps.ifaces
+ removeAssociationIfacesEntry(forwardPath + '/' + forwardType,
+ reversePath, assocMaps, server);
+ removeAssociationIfacesEntry(reversePath + '/' + reverseType,
+ forwardPath, assocMaps, server);
+
+ // Remove both sides of the association from assocMaps.owners
+ removeAssociationOwnersEntry(forwardPath + '/' + forwardType,
+ reversePath, owner, assocMaps, server);
+ removeAssociationOwnersEntry(reversePath + '/' + reverseType,
+ forwardPath, owner, assocMaps, server);
+ }
+}
diff --git a/src/associations.hpp b/src/associations.hpp
index 37f5e54..dd6cbdb 100644
--- a/src/associations.hpp
+++ b/src/associations.hpp
@@ -164,3 +164,18 @@
void findAssociations(const std::string& endpointPath,
AssociationMaps& assocMaps,
FindAssocResults& associationData);
+
+/** @brief If endpointPath is in an association, move that association
+ * to pending and remove the association objects.
+ *
+ * Called when a path is going off of D-Bus. If this path is an
+ * association endpoint (the path that owns the association is still
+ * on D-Bus), then move the association it's involved in to pending.
+ *
+ * @param[in] endpointPath - the D-Bus endpoint path to check
+ * @param[in,out] assocMaps - The association maps
+ * @param[in,out] server - sdbus system object
+ */
+void moveAssociationToPending(const std::string& endpointPath,
+ AssociationMaps& assocMaps,
+ sdbusplus::asio::object_server& server);
diff --git a/src/main.cpp b/src/main.cpp
index 84d5b67..1ae2c6c 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -558,11 +558,27 @@
}
interface_set->second.erase(interface);
- // If this was the last interface on this connection,
- // erase the connection
+
if (interface_set->second.empty())
{
+ // If this was the last interface on this connection,
+ // erase the connection
connection_map->second.erase(interface_set);
+
+ // Instead of checking if every single path is the endpoint
+ // of an association that needs to be moved to pending,
+ // only check when the only remaining owner of this path is
+ // ourself, which would be because we still own the
+ // association path.
+ if ((connection_map->second.size() == 1) &&
+ (connection_map->second.begin()->first ==
+ "xyz.openbmc_project.ObjectMapper"))
+ {
+ // Remove the 2 association D-Bus paths and move the
+ // association to pending.
+ moveAssociationToPending(obj_path.str, associationMaps,
+ server);
+ }
}
}
// If this was the last connection on this object path,
diff --git a/src/processing.cpp b/src/processing.cpp
index d3b7a76..6cc876d 100644
--- a/src/processing.cpp
+++ b/src/processing.cpp
@@ -70,6 +70,19 @@
{
removeAssociation(pathIt->first, wellKnown, server, assocMaps);
}
+
+ // Instead of checking if every single path is the endpoint of an
+ // association that needs to be moved to pending, only check when
+ // we own this path as well, which would be because of an
+ // association.
+ if ((pathIt->second.size() == 2) &&
+ (pathIt->second.find("xyz.openbmc_project.ObjectMapper") !=
+ pathIt->second.end()))
+ {
+ // Remove the 2 association D-Bus paths and move the
+ // association to pending.
+ moveAssociationToPending(pathIt->first, assocMaps, server);
+ }
}
pathIt->second.erase(wellKnown);
if (pathIt->second.empty())
diff --git a/src/test/associations.cpp b/src/test/associations.cpp
index 7ac2e79..50706d7 100644
--- a/src/test/associations.cpp
+++ b/src/test/associations.cpp
@@ -517,3 +517,49 @@
ASSERT_EQ(std::get<2>(a), "pathX");
}
}
+
+TEST_F(TestAssociations, moveAssocToPendingNoOp)
+{
+ AssociationMaps assocMaps;
+
+ // Not an association, so it shouldn't do anything
+ moveAssociationToPending(DEFAULT_ENDPOINT, assocMaps, *server);
+
+ EXPECT_TRUE(assocMaps.pending.empty());
+ EXPECT_TRUE(assocMaps.owners.empty());
+ EXPECT_TRUE(assocMaps.ifaces.empty());
+}
+
+TEST_F(TestAssociations, moveAssocToPending)
+{
+ AssociationMaps assocMaps;
+ assocMaps.owners = createDefaultOwnerAssociation();
+ assocMaps.ifaces = createDefaultInterfaceAssociation(server);
+
+ moveAssociationToPending(DEFAULT_ENDPOINT, assocMaps, *server);
+
+ // Check it's now pending
+ EXPECT_EQ(assocMaps.pending.size(), 1);
+ EXPECT_EQ(assocMaps.pending.begin()->first, DEFAULT_ENDPOINT);
+
+ // No more assoc owners
+ EXPECT_TRUE(assocMaps.owners.empty());
+
+ // Check the association interfaces were removed
+ {
+ auto assocs = assocMaps.ifaces.find(DEFAULT_FWD_PATH);
+ auto& iface = std::get<ifacePos>(assocs->second);
+ auto& endpoints = std::get<endpointsPos>(assocs->second);
+
+ EXPECT_EQ(iface.get(), nullptr);
+ EXPECT_TRUE(endpoints.empty());
+ }
+ {
+ auto assocs = assocMaps.ifaces.find(DEFAULT_REV_PATH);
+ auto& iface = std::get<ifacePos>(assocs->second);
+ auto& endpoints = std::get<endpointsPos>(assocs->second);
+
+ EXPECT_EQ(iface.get(), nullptr);
+ EXPECT_TRUE(endpoints.empty());
+ }
+}