LogService: Use DeleteAll DBus method in clearDump
Update the clearDump() implementation to call the DeleteAll D-Bus
method instead of iterating through D-Bus objects representing
individual log entries and calling the Delete D-Bus method on each one.
(It's more efficient for phosphor-debug-collector to iterate through
entries in its DeleteAll method handler than for bmcweb to iterate
through them.)
It seems like clearDump() wasn't originally implemented using DeleteAll
because dumps of various types were under the same D-Bus path namespace
at the time and there wasn't a way to selectively clear dumps of only a
specific type. The commit at [1] put different dump types under
different path namespaces (enabling us to now use DeleteAll).
Now clients should see a bit of performance improvement when running
the ClearLog action on dump LogServices, due to the reduced number of
D-Bus method calls needed to execute ClearLog.
Also updated getDumpServiceInfo() to populate the ClearLog action for
dump LogServices based on whether their dump manager object implements
xyz.openbmc_project.Collection.DeleteAll.
Tested:
Cleared the fault log containing 100 entries.
Ran with the time command several times before and after the change:
```
time curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Managers/bmc/LogServices/FaultLog/Actions/LogService.ClearLog
```
Before the change, "real" time reported was ~1.2s.
After the change, "real" time reported was ~0.4s.
Forced creation of dump entries and then ran Redfish ClearLog action on
each dump type:
```
curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Managers/bmc/LogServices/Dump/Actions/LogService.ClearLog
curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Managers/bmc/LogServices/FaultLog/Actions/LogService.ClearLog
curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Actions/LogService.ClearLog
```
Then verified that there were no dump LogService entries afterwards:
```
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Managers/bmc/LogServices/Dump/Entries
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Managers/bmc/LogServices/FaultLog/Entries
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Entries
```
Also verified that the corresponding D-Bus objects were gone from the
D-Bus tree after running ClearLog on each dump type:
Before ClearLog:
busctl tree xyz.openbmc_project.Dump.Manager
`-/xyz
`-/xyz/openbmc_project
`-/xyz/openbmc_project/dump
|-/xyz/openbmc_project/dump/bmc
| `-/xyz/openbmc_project/dump/bmc/entry
| `-/xyz/openbmc_project/dump/bmc/entry/101
|-/xyz/openbmc_project/dump/faultlog
| `-/xyz/openbmc_project/dump/faultlog/entry
| |-/xyz/openbmc_project/dump/faultlog/entry/11
| |-/xyz/openbmc_project/dump/faultlog/entry/12
| |-/xyz/openbmc_project/dump/faultlog/entry/13
| |-/xyz/openbmc_project/dump/faultlog/entry/14
| |-/xyz/openbmc_project/dump/faultlog/entry/15
| |-/xyz/openbmc_project/dump/faultlog/entry/16
| |-/xyz/openbmc_project/dump/faultlog/entry/17
| |-/xyz/openbmc_project/dump/faultlog/entry/18
| |-/xyz/openbmc_project/dump/faultlog/entry/19
| `-/xyz/openbmc_project/dump/faultlog/entry/20
|-/xyz/openbmc_project/dump/internal
| `-/xyz/openbmc_project/dump/internal/manager
`-/xyz/openbmc_project/dump/system
`-/xyz/openbmc_project/dump/system/entry
|-/xyz/openbmc_project/dump/system/entry/3
`-/xyz/openbmc_project/dump/system/entry/4
After ClearLog:
busctl tree xyz.openbmc_project.Dump.Manager
`-/xyz
`-/xyz/openbmc_project
`-/xyz/openbmc_project/dump
|-/xyz/openbmc_project/dump/bmc
|-/xyz/openbmc_project/dump/faultlog
|-/xyz/openbmc_project/dump/internal
| `-/xyz/openbmc_project/dump/internal/manager
`-/xyz/openbmc_project/dump/system
Confirmed that ClearLog action is listed for the following
LogServices:
/redfish/v1/Managers/bmc/LogServices/Dump
/redfish/v1/Managers/bmc/LogServices/FaultLog
/redfish/v1/Systems/system/LogServices/Dump
Then ran "systemctl stop xyz.openbmc_project.Dump.Manager" (which
removes dump manager objects including their
xyz.openbmc_project.Collection.DeleteAll interface) and saw that the
ClearLog action was no longer listed. Also locally built a version of
phosphor-debug-collecor with the interface
xyz.openbmc_project.Collection.DeleteAll removed from dump managers and
ran it and saw that the ClearLog action wasn't listed.
Redfish Service Validator passed on the following URIs
(with service xyz.openbmc_project.Dump.Manager running):
/redfish/v1/Managers/bmc/LogServices/Dump
/redfish/v1/Managers/bmc/LogServices/FaultLog
/redfish/v1/Systems/system/LogServices/Dump
Note: Most dump LogService unit tests were removed in this patchset
since this patchset adds a D-Bus call to getDumpServiceInfo(), and
we haven't decided how to mock D-Bus calls for unit testing yet.
[1] https://github.com/openbmc/phosphor-debug-collector/commit/fef66a951fe6fe283515480b2c493dfdc2275a95
Signed-off-by: Claire Weinan <cweinan@google.com>
Change-Id: Ic5f8f9e3528f521887766d8710bd77f969d8236a
diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp
index 09b2155..2f3c8ab 100644
--- a/redfish-core/lib/log_services.hpp
+++ b/redfish-core/lib/log_services.hpp
@@ -974,32 +974,19 @@
{
std::string dumpTypeLowerCopy =
std::string(boost::algorithm::to_lower_copy(dumpType));
- std::string interface = "xyz.openbmc_project.Dump.Entry." + dumpType;
- const std::array<const std::string_view, 1> interfaces{interface};
- dbus::utility::getSubTreePaths(
- "/xyz/openbmc_project/dump/" + dumpTypeLowerCopy, 0, interfaces,
- [asyncResp, dumpType](
- const boost::system::error_code& ec,
- const dbus::utility::MapperGetSubTreePathsResponse& subTreePaths) {
+ crow::connections::systemBus->async_method_call(
+ [asyncResp](const boost::system::error_code& ec) {
if (ec)
{
- BMCWEB_LOG_ERROR << "resp_handler got error " << ec;
+ BMCWEB_LOG_ERROR << "clearDump resp_handler got error " << ec;
messages::internalError(asyncResp->res);
return;
}
-
- for (const std::string& path : subTreePaths)
- {
- sdbusplus::message::object_path objPath(path);
- std::string logID = objPath.filename();
- if (logID.empty())
- {
- continue;
- }
- deleteDumpEntry(asyncResp, logID, dumpType);
- }
- });
+ },
+ "xyz.openbmc_project.Dump.Manager",
+ "/xyz/openbmc_project/dump/" + dumpTypeLowerCopy,
+ "xyz.openbmc_project.Collection.DeleteAll", "DeleteAll");
}
inline static void
@@ -2632,8 +2619,6 @@
redfishDateTimeOffset.second;
asyncResp->res.jsonValue["Entries"]["@odata.id"] = dumpPath + "/Entries";
- asyncResp->res.jsonValue["Actions"]["#LogService.ClearLog"]["target"] =
- dumpPath + "/Actions/LogService.ClearLog";
if (collectDiagnosticDataSupported)
{
@@ -2641,6 +2626,37 @@
["target"] =
dumpPath + "/Actions/LogService.CollectDiagnosticData";
}
+
+ constexpr std::array<std::string_view, 1> interfaces = {deleteAllInterface};
+ dbus::utility::getSubTreePaths(
+ "/xyz/openbmc_project/dump", 0, interfaces,
+ [asyncResp, dumpType, dumpPath](
+ const boost::system::error_code& ec,
+ const dbus::utility::MapperGetSubTreePathsResponse& subTreePaths) {
+ if (ec)
+ {
+ BMCWEB_LOG_ERROR << "getDumpServiceInfo respHandler got error "
+ << ec;
+ // Assume that getting an error simply means there are no dump
+ // LogServices. Return without adding any error response.
+ return;
+ }
+
+ const std::string dbusDumpPath =
+ "/xyz/openbmc_project/dump/" +
+ boost::algorithm::to_lower_copy(dumpType);
+
+ for (const std::string& path : subTreePaths)
+ {
+ if (path == dbusDumpPath)
+ {
+ asyncResp->res
+ .jsonValue["Actions"]["#LogService.ClearLog"]["target"] =
+ dumpPath + "/Actions/LogService.ClearLog";
+ break;
+ }
+ }
+ });
}
inline void handleLogServicesDumpServiceGet(