PEL: Watch for manually deleted PEL files
Currently, if someone were to manually delete a PEL file, the Repository
class would not know to remove it from its _pelAttributes index which
will lead to problems down the line.
To fix this, create an inotify watch that looks for files to be deleted
in the PEL directory. When one or more is deleted, remove it from the
Repository class and also remove the OpenBMC event log for it.
Signed-off-by: Matt Spinler <spinler@us.ibm.com>
Change-Id: I238c0b886b01e88cc931162a0596b848d1d975b1
diff --git a/extensions/openpower-pels/manager.cpp b/extensions/openpower-pels/manager.cpp
index fea2ba9..d7fbcad 100644
--- a/extensions/openpower-pels/manager.cpp
+++ b/extensions/openpower-pels/manager.cpp
@@ -19,6 +19,7 @@
#include "json_utils.hpp"
#include "pel.hpp"
+#include <sys/inotify.h>
#include <unistd.h>
#include <filesystem>
@@ -45,6 +46,18 @@
constexpr auto esel = "ESEL";
} // namespace additional_data
+Manager::~Manager()
+{
+ if (_pelFileDeleteFD != -1)
+ {
+ if (_pelFileDeleteWatchFD != -1)
+ {
+ inotify_rm_watch(_pelFileDeleteFD, _pelFileDeleteWatchFD);
+ }
+ close(_pelFileDeleteFD);
+ }
+}
+
void Manager::create(const std::string& message, uint32_t obmcLogID,
uint64_t timestamp, Entry::Level severity,
const std::vector<std::string>& additionalData,
@@ -341,11 +354,9 @@
void Manager::scheduleFDClose(int fd)
{
- sdeventplus::Event event = sdeventplus::Event::get_default();
-
_fdCloserEventSource = std::make_unique<sdeventplus::source::Defer>(
- event, std::bind(std::mem_fn(&Manager::closeFD), this, fd,
- std::placeholders::_1));
+ _event, std::bind(std::mem_fn(&Manager::closeFD), this, fd,
+ std::placeholders::_1));
}
void Manager::closeFD(int fd, sdeventplus::source::EventBase& source)
@@ -426,11 +437,9 @@
void Manager::scheduleRepoPrune()
{
- sdeventplus::Event event = sdeventplus::Event::get_default();
-
_repoPrunerEventSource = std::make_unique<sdeventplus::source::Defer>(
- event, std::bind(std::mem_fn(&Manager::pruneRepo), this,
- std::placeholders::_1));
+ _event, std::bind(std::mem_fn(&Manager::pruneRepo), this,
+ std::placeholders::_1));
}
void Manager::pruneRepo(sdeventplus::source::EventBase& source)
@@ -444,5 +453,95 @@
_repoPrunerEventSource.reset();
}
+void Manager::setupPELDeleteWatch()
+{
+ _pelFileDeleteFD = inotify_init1(IN_NONBLOCK);
+ if (-1 == _pelFileDeleteFD)
+ {
+ auto e = errno;
+ std::string msg =
+ "inotify_init1 failed with errno " + std::to_string(e);
+ log<level::ERR>(msg.c_str());
+ abort();
+ }
+
+ _pelFileDeleteWatchFD = inotify_add_watch(
+ _pelFileDeleteFD, _repo.repoPath().c_str(), IN_DELETE);
+ if (-1 == _pelFileDeleteWatchFD)
+ {
+ auto e = errno;
+ std::string msg =
+ "inotify_add_watch failed with error " + std::to_string(e);
+ log<level::ERR>(msg.c_str());
+ abort();
+ }
+
+ _pelFileDeleteEventSource = std::make_unique<sdeventplus::source::IO>(
+ _event, _pelFileDeleteFD, EPOLLIN,
+ std::bind(std::mem_fn(&Manager::pelFileDeleted), this,
+ std::placeholders::_1, std::placeholders::_2,
+ std::placeholders::_3));
+}
+
+void Manager::pelFileDeleted(sdeventplus::source::IO& io, int fd,
+ uint32_t revents)
+{
+ if (!(revents & EPOLLIN))
+ {
+ return;
+ }
+
+ // An event for 1 PEL uses 48B. When all PELs are deleted at once,
+ // as many events as there is room for can be handled in one callback.
+ // A size of 2000 will allow 41 to be processed, with additional
+ // callbacks being needed to process the remaining ones.
+ std::array<uint8_t, 2000> data;
+ auto bytesRead = read(_pelFileDeleteFD, data.data(), data.size());
+ if (bytesRead < 0)
+ {
+ auto e = errno;
+ std::string msg = "Failed reading data from inotify event, errno = " +
+ std::to_string(e);
+ log<level::ERR>(msg.c_str());
+ abort();
+ }
+
+ auto offset = 0;
+ while (offset < bytesRead)
+ {
+ auto event = reinterpret_cast<inotify_event*>(&data[offset]);
+ if (event->mask & IN_DELETE)
+ {
+ std::string filename{event->name};
+
+ // Get the PEL ID from the filename and tell the
+ // repo it's been removed, and then delete the BMC
+ // event log if it's there.
+ auto pos = filename.find_first_of('_');
+ if (pos != std::string::npos)
+ {
+ try
+ {
+ auto idString = filename.substr(pos + 1);
+ auto pelID = std::stoul(idString, nullptr, 16);
+
+ Repository::LogID id{Repository::LogID::Pel(pelID)};
+ auto removedLogID = _repo.remove(id);
+ if (removedLogID)
+ {
+ _logManager.erase(removedLogID->obmcID.id);
+ }
+ }
+ catch (const std::exception& e)
+ {
+ log<level::INFO>("Could not find PEL ID from its filename",
+ entry("FILENAME=%s", filename.c_str()));
+ }
+ }
+ }
+
+ offset += offsetof(inotify_event, name) + event->len;
+ }
+}
} // namespace pels
} // namespace openpower
diff --git a/extensions/openpower-pels/manager.hpp b/extensions/openpower-pels/manager.hpp
index 2d443c6..50317a5 100644
--- a/extensions/openpower-pels/manager.hpp
+++ b/extensions/openpower-pels/manager.hpp
@@ -31,7 +31,6 @@
{
public:
Manager() = delete;
- ~Manager() = default;
Manager(const Manager&) = default;
Manager& operator=(const Manager&) = default;
Manager(Manager&&) = default;
@@ -52,8 +51,10 @@
_logManager(logManager), _eventLogger(std::move(creatorFunc)),
_repo(getPELRepoPath()),
_registry(getPELReadOnlyDataPath() / message::registryFileName),
+ _event(sdeventplus::Event::get_default()),
_dataIface(std::move(dataIface))
{
+ setupPELDeleteWatch();
}
/**
@@ -76,6 +77,11 @@
}
/**
+ * @brief Destructor
+ */
+ ~Manager();
+
+ /**
* @brief Creates a PEL based on the OpenBMC event log contents. If
* a PEL was passed in via the RAWPEL specifier in the
* additionalData parameter, use that instead.
@@ -271,6 +277,21 @@
void pruneRepo(sdeventplus::source::EventBase& source);
/**
+ * @brief Sets up an inotify watch to watch for deleted PEL
+ * files. Calls pelFileDeleted() when that occurs.
+ */
+ void setupPELDeleteWatch();
+
+ /**
+ * @brief Called when the inotify watch put on the repository directory
+ * detects a PEL file was deleted.
+ *
+ * Will tell the Repository class about the deleted PEL, and then tell
+ * the log manager class to delete the corresponding OpenBMC event log.
+ */
+ void pelFileDeleted(sdeventplus::source::IO& io, int fd, uint32_t revents);
+
+ /**
* @brief Reference to phosphor-logging's Manager class
*/
phosphor::logging::internal::Manager& _logManager;
@@ -292,6 +313,11 @@
message::Registry _registry;
/**
+ * @brief The Event object this class uses
+ */
+ sdeventplus::Event _event;
+
+ /**
* @brief The API the PEL sections use to gather data
*/
std::unique_ptr<DataInterfaceBase> _dataIface;
@@ -313,6 +339,22 @@
* running out of space to make room for new ones.
*/
std::unique_ptr<sdeventplus::source::Defer> _repoPrunerEventSource;
+
+ /**
+ * @brief The even source for watching for deleted PEL files.
+ */
+ std::unique_ptr<sdeventplus::source::IO> _pelFileDeleteEventSource;
+
+ /**
+ * @brief The file descriptor returned by inotify_init1() used
+ * for watching for deleted PEL files.
+ */
+ int _pelFileDeleteFD = -1;
+
+ /**
+ * @brief The file descriptor returned by inotify_add_watch().
+ */
+ int _pelFileDeleteWatchFD = -1;
};
} // namespace pels
diff --git a/extensions/openpower-pels/repository.hpp b/extensions/openpower-pels/repository.hpp
index 96f951c..9bb71e4 100644
--- a/extensions/openpower-pels/repository.hpp
+++ b/extensions/openpower-pels/repository.hpp
@@ -404,6 +404,17 @@
*/
std::vector<uint32_t> prune();
+ /**
+ * @brief Returns the path to the directory where the PEL
+ * files are stored.
+ *
+ * @return std::filesystem::path - The directory path
+ */
+ const std::filesystem::path& repoPath() const
+ {
+ return _logPath;
+ }
+
private:
using PELUpdateFunc = std::function<void(PEL&)>;
diff --git a/test/openpower-pels/pel_manager_test.cpp b/test/openpower-pels/pel_manager_test.cpp
index bbc89a6..b7146c9 100644
--- a/test/openpower-pels/pel_manager_test.cpp
+++ b/test/openpower-pels/pel_manager_test.cpp
@@ -105,6 +105,23 @@
return count;
}
+void deletePELFile(uint32_t id)
+{
+ char search[20];
+
+ sprintf(search, "\\d+_%.8X", id);
+ std::regex expr{search};
+
+ for (auto& f : fs::directory_iterator(getPELRepoPath() / "logs"))
+ {
+ if (std::regex_search(f.path().string(), expr))
+ {
+ fs::remove(f.path());
+ break;
+ }
+ }
+}
+
// Test that using the RAWPEL=<file> with the Manager::create() call gets
// a PEL saved in the repository.
TEST_F(ManagerTest, TestCreateWithPEL)
@@ -609,3 +626,145 @@
fs::remove_all(dir);
}
+
+// Test that manually deleting a PEL file will be recognized by the code.
+TEST_F(ManagerTest, TestPELManualDelete)
+{
+ sdeventplus::Event e{sdEvent};
+
+ std::unique_ptr<DataInterfaceBase> dataIface =
+ std::make_unique<MockDataInterface>();
+
+ openpower::pels::Manager manager{
+ logManager, std::move(dataIface),
+ std::bind(std::mem_fn(&TestLogger::log), &logger, std::placeholders::_1,
+ std::placeholders::_2, std::placeholders::_3)};
+
+ auto data = pelDataFactory(TestPELType::pelSimple);
+ auto dir = makeTempDir();
+ fs::path pelFilename = dir / "rawpel";
+
+ std::string adItem = "RAWPEL=" + pelFilename.string();
+ std::vector<std::string> additionalData{adItem};
+ std::vector<std::string> associations;
+
+ // Add 20 PELs, they will get incrementing IDs like
+ // 0x50000001, 0x50000002, etc.
+ for (int i = 1; i <= 20; i++)
+ {
+ std::ofstream pelFile{pelFilename};
+ pelFile.write(reinterpret_cast<const char*>(data.data()), data.size());
+ pelFile.close();
+
+ manager.create("error message", 42, 0,
+ phosphor::logging::Entry::Level::Error, additionalData,
+ associations);
+
+ // Sanity check this ID is really there so we can test
+ // it was deleted later. This will throw an exception if
+ // not present.
+ manager.getPEL(0x50000000 + i);
+
+ // Run an event loop pass where the internal FD is deleted
+ // after the getPEL function call.
+ e.run(std::chrono::milliseconds(1));
+ }
+
+ EXPECT_EQ(countPELsInRepo(), 20);
+
+ deletePELFile(0x50000001);
+
+ // Run a single event loop pass so the inotify event can run
+ e.run(std::chrono::milliseconds(1));
+
+ EXPECT_EQ(countPELsInRepo(), 19);
+
+ EXPECT_THROW(
+ manager.getPEL(0x50000001),
+ sdbusplus::xyz::openbmc_project::Common::Error::InvalidArgument);
+
+ // Delete a few more, they should all get handled in the same
+ // event loop pass
+ std::vector<uint32_t> toDelete{0x50000002, 0x50000003, 0x50000004,
+ 0x50000005, 0x50000006};
+ std::for_each(toDelete.begin(), toDelete.end(),
+ [](auto i) { deletePELFile(i); });
+
+ e.run(std::chrono::milliseconds(1));
+
+ EXPECT_EQ(countPELsInRepo(), 14);
+
+ std::for_each(toDelete.begin(), toDelete.end(), [&manager](const auto i) {
+ EXPECT_THROW(
+ manager.getPEL(i),
+ sdbusplus::xyz::openbmc_project::Common::Error::InvalidArgument);
+ });
+
+ fs::remove_all(dir);
+}
+
+// Test that deleting all PELs at once is handled OK.
+TEST_F(ManagerTest, TestPELManualDeleteAll)
+{
+ sdeventplus::Event e{sdEvent};
+
+ std::unique_ptr<DataInterfaceBase> dataIface =
+ std::make_unique<MockDataInterface>();
+
+ openpower::pels::Manager manager{
+ logManager, std::move(dataIface),
+ std::bind(std::mem_fn(&TestLogger::log), &logger, std::placeholders::_1,
+ std::placeholders::_2, std::placeholders::_3)};
+
+ auto data = pelDataFactory(TestPELType::pelSimple);
+ auto dir = makeTempDir();
+ fs::path pelFilename = dir / "rawpel";
+
+ std::string adItem = "RAWPEL=" + pelFilename.string();
+ std::vector<std::string> additionalData{adItem};
+ std::vector<std::string> associations;
+
+ // Add 200 PELs, they will get incrementing IDs like
+ // 0x50000001, 0x50000002, etc.
+ for (int i = 1; i <= 200; i++)
+ {
+ std::ofstream pelFile{pelFilename};
+ pelFile.write(reinterpret_cast<const char*>(data.data()), data.size());
+ pelFile.close();
+
+ manager.create("error message", 42, 0,
+ phosphor::logging::Entry::Level::Error, additionalData,
+ associations);
+
+ // Sanity check this ID is really there so we can test
+ // it was deleted later. This will throw an exception if
+ // not present.
+ manager.getPEL(0x50000000 + i);
+
+ // Run an event loop pass where the internal FD is deleted
+ // after the getPEL function call.
+ e.run(std::chrono::milliseconds(1));
+ }
+
+ // Delete them all at once
+ auto logPath = getPELRepoPath() / "logs";
+ std::string cmd = "rm " + logPath.string() + "/*";
+ system(cmd.c_str());
+
+ EXPECT_EQ(countPELsInRepo(), 0);
+
+ // It will take 5 event loop passes to process them all
+ for (int i = 0; i < 5; i++)
+ {
+ e.run(std::chrono::milliseconds(1));
+ }
+
+ for (int i = 1; i <= 200; i++)
+ {
+ EXPECT_THROW(
+ manager.getPEL(0x50000000 + i),
+ sdbusplus::xyz::openbmc_project::Common::Error::InvalidArgument);
+ }
+
+ fs::remove_all(dir);
+}