commit 9c495c62baa219f3cc60935cd5a46657846d7232
author Jonico Eustaquio <jonico.eustaquio@fii-na.com> Tue Jul 02 16:35:14 2024 -0500
committer Jonico Eustaquio <jonico.eustaquio@fii-na.com> Mon Sep 23 21:32:20 2024 +0000
tree 672e6d57ec765ed3fefad3bc3a5f87977c2dd8fa
parent 483ae8f09371cadeda4be02d8c109ac5dae5a98e

Change selDelete to return void to avoid magic numbers

The selDelete function is changed to throw a ResourceNotFound error
when attempting to delete a non-existent SEL record instead of returning
a magic number to signal phosphor-ipmi-host. Additionally, the
phosphor-ipmi-host package will call the SELDelete method to call
selDelete function so the dbus method name has been updated to reflect
that.

Tested:
1. Generated SEL entries
2. Delete a SEL entry using:
   busctl call xyz.openbmc_project.Logging.IPMI \
   /xyz/openbmc_project/Logging/IPMI xyz.openbmc_project.Logging.IPMI \
   SELDelete q <RecordID>
3. Using ipmitool sel list, all remaining SEL entries are seen
4. See a ResourceNotFound error if used a RecordID not already in SEL

Change-Id: I33f0b70e9fbe4c7d8dc0f35589ae3096ed5fe269
Signed-off-by: Jonico Eustaquio <jonico.eustaquio@fii-na.com>

meson.build

diff --git a/meson.build b/meson.build
index a3a1216..d382a49 100644
--- a/meson.build
+++ b/meson.build
@@ -39,6 +39,8 @@
 endif
 if get_option('sel-delete')
   cpp_args += '-DSEL_LOGGER_ENABLE_SEL_DELETE'
+
+  deps += dependency('phosphor-dbus-interfaces')
 endif
 
 executable(

meson.options

diff --git a/meson.options b/meson.options
index 39f082d..0ca3214 100644
--- a/meson.options
+++ b/meson.options
@@ -11,4 +11,4 @@
 option('send-to-logger', type: 'boolean',
        description: 'Automatically log events to Redfish for pulse type assert-deassert sensor events')
 option('sel-delete', type: 'boolean',
-       description: 'Enables ability to delete SEL entries given a record ID')
\ No newline at end of file
+       description: 'Enables ability to delete SEL entries given a record ID')

src/sel_logger.cpp

diff --git a/src/sel_logger.cpp b/src/sel_logger.cpp
index 2c1d1fe..493b443 100644
--- a/src/sel_logger.cpp
+++ b/src/sel_logger.cpp
@@ -24,6 +24,9 @@
 #include <sel_logger.hpp>
 #include <threshold_event_monitor.hpp>
 #include <watchdog_event_monitor.hpp>
+#ifdef SEL_LOGGER_ENABLE_SEL_DELETE
+#include <xyz/openbmc_project/Common/error.hpp>
+#endif
 #ifdef SEL_LOGGER_MONITOR_THRESHOLD_ALARM_EVENTS
 #include <threshold_alarm_event_monitor.hpp>
 #endif
@@ -245,7 +248,7 @@
     return targetEntryFound;
 }
 
-static uint16_t selDeleteRecord(const uint16_t& recordId)
+static void selDeleteRecord(const uint16_t& recordId)
 {
     std::filesystem::file_time_type prevAddTime =
         std::filesystem::last_write_time(selLogDir / selLogFilename);
@@ -254,7 +257,8 @@
     // Check if the Record Id was found
     if (!targetEntryFound)
     {
-        return selInvalidRecID;
+        throw sdbusplus::xyz::openbmc_project::Common::Error::
+            ResourceNotFound();
     }
     // Add to next record cache for reuse
     nextRecordsCache.push_back(recordId);
@@ -266,7 +270,6 @@
     std::filesystem::last_write_time(selLogDir / selLogFilename, prevAddTime);
     // Update Last Del Time
     saveClearSelTimestamp();
-    return recordId;
 }
 #else
 static unsigned int initializeRecordId()
@@ -497,7 +500,7 @@
     ifaceAddSel->register_method("Clear", []() { clearSelLogFiles(); });
 #ifdef SEL_LOGGER_ENABLE_SEL_DELETE
     // Delete a SEL entry
-    ifaceAddSel->register_method("IpmiSelDelete", [](const uint16_t& recordId) {
+    ifaceAddSel->register_method("SELDelete", [](const uint16_t& recordId) {
         return selDeleteRecord(recordId);
     });
 #endif