PEL: Prohibit PEL resolving if associated guard record exists
The commit prohibits marking the 'Resolve' flag of the PEL as 'true'
until the associated guard record resolved if one exists.The user will
get notified with the same by throwing the DBUS error
`sdbusplus::xyz::openbmc_project::Common::Error::Unavailable`.
Implemented approach:
The PELs will be prohibited from deleting also if the PEL is
associated with a guard record. Hence, implementation is like that
resolve will be prohibited until delete does the same.
Tested:
Sample output:
```
busctl set-property xyz.openbmc_project.Logging
/xyz/openbmc_project/logging/entry/406 xyz.openbmc_project.Logging.Entry
Resolved b true
Failed to set property Resolved on interface xyz.openbmc_project.Logging.Entry:
The service is temporarily unavailable.
```
Change-Id: I77cc191e95c494423d3c875c231fdace8237cf22
Signed-off-by: Arya K Padman<aryakpadman@gmail.com>
diff --git a/elog_entry.cpp b/elog_entry.cpp
index b4009a5..183ecbb 100644
--- a/elog_entry.cpp
+++ b/elog_entry.cpp
@@ -1,6 +1,7 @@
#include "elog_entry.hpp"
#include "elog_serialize.hpp"
+#include "extensions.hpp"
#include "log_manager.hpp"
#include <fcntl.h>
@@ -27,6 +28,33 @@
sdbusplus::server::xyz::openbmc_project::logging::Entry::resolved();
if (value != current)
{
+ // Resolve operation will be prohibited if delete operation is
+ // prohibited.
+ for (auto& func : Extensions::getDeleteProhibitedFunctions())
+ {
+ try
+ {
+ bool prohibited = false;
+ func(id(), prohibited);
+
+ if (prohibited)
+ {
+ throw sdbusplus::xyz::openbmc_project::Common::Error::
+ Unavailable();
+ }
+ }
+ catch (const sdbusplus::xyz::openbmc_project::Common::Error::
+ Unavailable& e)
+ {
+ throw;
+ }
+ catch (const std::exception& e)
+ {
+ lg2::error("An extension's deleteProhibited function threw an "
+ "exception: {ERROR}",
+ "ERROR", e);
+ }
+ }
value ? associations({}) : associations(assocs);
current =
sdbusplus::server::xyz::openbmc_project::logging::Entry::resolved(
diff --git a/extensions.hpp b/extensions.hpp
index ff74137..727e288 100644
--- a/extensions.hpp
+++ b/extensions.hpp
@@ -45,6 +45,10 @@
/**
* @brief The function type that will to check if an event log is prohibited
* from being deleted.
+ * The same function is used to check if an event log is prohibited from
+ * setting Resolved flag, as Resolve is prohibited as long as Delete is
+ * prohibited.
+ *
* @param[in] uint32_t - The event log ID
* @param[out] bool - set to true if the delete is prohibited
*/
diff --git a/test/elog_update_ts_test.cpp b/test/elog_update_ts_test.cpp
index d04da04..07d9613 100644
--- a/test/elog_update_ts_test.cpp
+++ b/test/elog_update_ts_test.cpp
@@ -2,6 +2,7 @@
#include "elog_entry.hpp"
#include "elog_serialize.hpp"
+#include "extensions.hpp"
#include "log_manager.hpp"
#include <filesystem>
@@ -19,6 +20,16 @@
using namespace std::chrono_literals;
namespace fs = std::filesystem;
+void deleteIsProhibitedMock(uint32_t /*id*/, bool& prohibited)
+{
+ prohibited = true;
+}
+
+void deleteIsNotProhibitedMock(uint32_t /*id*/, bool& prohibited)
+{
+ prohibited = false;
+}
+
// Test that the update timestamp changes when the resolved property changes
TEST(TestUpdateTS, testChangeResolved)
{
@@ -88,6 +99,65 @@
fs::remove(fs::path{ERRLOG_PERSIST_PATH} / std::to_string(id));
}
+TEST(TestResolveProhibited, testResolveFlagChange)
+{
+ // Setting resolved will serialize, so need this directory.
+ fs::create_directory(ERRLOG_PERSIST_PATH);
+
+ if (!fs::exists(ERRLOG_PERSIST_PATH))
+ {
+ ADD_FAILURE() << "Could not create " << ERRLOG_PERSIST_PATH << "\n";
+ exit(1);
+ }
+
+ auto bus = sdbusplus::bus::new_default();
+ phosphor::logging::internal::Manager manager(bus, OBJ_INTERNAL);
+
+ // Use a random number for the ID to avoid other CI
+ // testcases running in parallel.
+ std::srand(std::time(nullptr));
+ uint32_t id = std::rand();
+
+ if (fs::exists(fs::path{ERRLOG_PERSIST_PATH} / std::to_string(id)))
+ {
+ std::cerr << "Another testcase is using ID " << id << "\n";
+ id = std::rand();
+ }
+
+ uint64_t timestamp{100};
+ std::string message{"test error"};
+ std::string fwLevel{"level42"};
+ std::string path{"/tmp/99"};
+ std::vector<std::string> testData{"additional", "data"};
+ phosphor::logging::AssociationList associations{};
+
+ Entry elog{bus,
+ std::string(OBJ_ENTRY) + '/' + std::to_string(id),
+ id,
+ timestamp,
+ Entry::Level::Informational,
+ std::move(message),
+ std::move(testData),
+ std::move(associations),
+ fwLevel,
+ path,
+ manager};
+
+ Extensions ext{deleteIsProhibitedMock};
+
+ EXPECT_THROW(elog.resolved(true),
+ sdbusplus::xyz::openbmc_project::Common::Error::Unavailable);
+
+ Extensions::getDeleteProhibitedFunctions().clear();
+
+ Extensions e{deleteIsNotProhibitedMock};
+
+ EXPECT_NO_THROW(elog.resolved(true));
+ EXPECT_EQ(elog.resolved(), true);
+
+ // Leave the directory in case other CI instances are running
+ fs::remove(fs::path{ERRLOG_PERSIST_PATH} / std::to_string(id));
+}
} // namespace test
} // namespace logging
} // namespace phosphor