regulators: Implement D-Bus error logging
Implemented the DBusErrorLogging class. This class creates error logs
using the D-Bus CreateWithFFDCFiles method.
Updated the abstract base class, ErrorLogging, to have the correct
virtual method parameters. The methods were previously defined with no
parameters as a temporary measure.
Also updated the MockErrorLogging class to have the correct virtual
method parameters.
Tested:
* Verified that all the log*Error() methods create an error log of the
correct type.
* Verified that created error logs have the expected:
* property/field values
* callouts with associated VPD
* User Data sections containing debug data stored in FFDC files
* Tested where creating an FFDC file fails.
* Tested where calling CreateWithFFDCFiles method fails.
* Tested where removing an FFDC file fails.
* Verified that if a failure occurs, it is written to the system
journal but does not result in a second error log (since that could
lead to an infinite loop).
* Verified that temporary FFDC files are removed even if creating the
error log fails.
Full Test Plan:
* https://gist.github.com/smccarney/60ecbc018c55a5d13661bda8ee256d61
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
Change-Id: I2837fc68dfbad2d89193a147222f1c51d9b1aad3
diff --git a/phosphor-regulators/src/error_logging.cpp b/phosphor-regulators/src/error_logging.cpp
new file mode 100644
index 0000000..37f00a9
--- /dev/null
+++ b/phosphor-regulators/src/error_logging.cpp
@@ -0,0 +1,251 @@
+/**
+ * Copyright © 2020 IBM Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "error_logging.hpp"
+
+#include "exception_utils.hpp"
+
+#include <errno.h> // for errno
+#include <string.h> // for strerror()
+#include <sys/types.h> // for getpid(), lseek(), ssize_t
+#include <unistd.h> // for getpid(), lseek(), write()
+
+#include <sdbusplus/message.hpp>
+
+#include <exception>
+#include <ios>
+#include <sstream>
+#include <stdexcept>
+
+namespace phosphor::power::regulators
+{
+
+void DBusErrorLogging::logConfigFileError(Entry::Level severity,
+ Journal& journal)
+{
+ std::map<std::string, std::string> additionalData{};
+ logError("xyz.openbmc_project.Power.Regulators.Error.ConfigFile", severity,
+ additionalData, journal);
+}
+
+void DBusErrorLogging::logDBusError(Entry::Level severity, Journal& journal)
+{
+ std::map<std::string, std::string> additionalData{};
+ logError("xyz.openbmc_project.Power.Error.DBus", severity, additionalData,
+ journal);
+}
+
+void DBusErrorLogging::logI2CError(Entry::Level severity, Journal& journal,
+ const std::string& bus, uint8_t addr,
+ int errorNumber)
+{
+ // Convert I2C address to a hex string
+ std::ostringstream ss;
+ ss << "0x" << std::hex << std::uppercase << static_cast<uint16_t>(addr);
+ std::string addrStr = ss.str();
+
+ // Convert errno value to an integer string
+ std::string errorNumberStr = std::to_string(errorNumber);
+
+ std::map<std::string, std::string> additionalData{};
+ additionalData.emplace("CALLOUT_IIC_BUS", bus);
+ additionalData.emplace("CALLOUT_IIC_ADDR", addrStr);
+ additionalData.emplace("CALLOUT_ERRNO", errorNumberStr);
+ logError("xyz.openbmc_project.Power.Error.I2C", severity, additionalData,
+ journal);
+}
+
+void DBusErrorLogging::logInternalError(Entry::Level severity, Journal& journal)
+{
+ std::map<std::string, std::string> additionalData{};
+ logError("xyz.openbmc_project.Power.Error.Internal", severity,
+ additionalData, journal);
+}
+
+void DBusErrorLogging::logPMBusError(Entry::Level severity, Journal& journal,
+ const std::string& inventoryPath)
+{
+ // Convert relative inventory path to an absolute path
+ std::string absInventoryPath = getAbsoluteInventoryPath(inventoryPath);
+
+ std::map<std::string, std::string> additionalData{};
+ additionalData.emplace("CALLOUT_INVENTORY_PATH", absInventoryPath);
+ logError("xyz.openbmc_project.Power.Error.PMBus", severity, additionalData,
+ journal);
+}
+
+void DBusErrorLogging::logWriteVerificationError(
+ Entry::Level severity, Journal& journal, const std::string& inventoryPath)
+{
+ // Convert relative inventory path to an absolute path
+ std::string absInventoryPath = getAbsoluteInventoryPath(inventoryPath);
+
+ std::map<std::string, std::string> additionalData{};
+ additionalData.emplace("CALLOUT_INVENTORY_PATH", absInventoryPath);
+ logError("xyz.openbmc_project.Power.Regulators.Error.WriteVerification",
+ severity, additionalData, journal);
+}
+
+FFDCFile DBusErrorLogging::createFFDCFile(const std::vector<std::string>& lines)
+{
+ // Create FFDC file of type Text
+ FFDCFile file{FFDCFormat::Text};
+ int fd = file.getFileDescriptor();
+
+ // Write lines to file
+ std::string buffer;
+ for (const std::string& line : lines)
+ {
+ // Copy line to buffer. Add newline if necessary.
+ buffer = line;
+ if (line.empty() || (line.back() != '\n'))
+ {
+ buffer += '\n';
+ }
+
+ // Write buffer to file
+ const char* bufPtr = buffer.c_str();
+ unsigned int count = buffer.size();
+ while (count > 0)
+ {
+ // Try to write remaining bytes; it might not write all of them
+ ssize_t bytesWritten = write(fd, bufPtr, count);
+ if (bytesWritten == -1)
+ {
+ throw std::runtime_error{
+ std::string{"Unable to write to FFDC file: "} +
+ strerror(errno)};
+ }
+ bufPtr += bytesWritten;
+ count -= bytesWritten;
+ }
+ }
+
+ // Seek to beginning of file so error logging system can read data
+ if (lseek(fd, 0, SEEK_SET) != 0)
+ {
+ throw std::runtime_error{
+ std::string{"Unable to seek within FFDC file: "} + strerror(errno)};
+ }
+
+ return file;
+}
+
+std::vector<FFDCFile> DBusErrorLogging::createFFDCFiles(Journal& journal)
+{
+ std::vector<FFDCFile> files{};
+
+ // Create FFDC files containing journal messages from relevant executables.
+ // Executables in priority order in case error log cannot hold all the FFDC.
+ std::vector<std::string> executables{"phosphor-regulators", "systemd"};
+ for (const std::string& executable : executables)
+ {
+ try
+ {
+ // Get recent journal messages from the executable
+ // TODO: Uncomment the following line and remove the temporary code
+ // when Journal::getMessages() is implemented
+ // std::vector<std::string> messages =
+ // journal.getMessages("SYSLOG_IDENTIFIER", executable, 30);
+ std::vector<std::string> messages{
+ executable + ": journal message 1",
+ executable + ": journal message 2"};
+
+ // Create FFDC file containing the journal messages
+ if (!messages.empty())
+ {
+ files.emplace_back(createFFDCFile(messages));
+ }
+ }
+ catch (const std::exception& e)
+ {
+ journal.logError(exception_utils::getMessages(e));
+ }
+ }
+
+ return files;
+}
+
+std::vector<FFDCTuple>
+ DBusErrorLogging::createFFDCTuples(std::vector<FFDCFile>& files)
+{
+ std::vector<FFDCTuple> ffdcTuples{};
+ for (FFDCFile& file : files)
+ {
+ ffdcTuples.emplace_back(
+ file.getFormat(), file.getSubType(), file.getVersion(),
+ sdbusplus::message::unix_fd(file.getFileDescriptor()));
+ }
+ return ffdcTuples;
+}
+
+void DBusErrorLogging::logError(
+ const std::string& message, Entry::Level severity,
+ std::map<std::string, std::string>& additionalData, Journal& journal)
+{
+ try
+ {
+ // Add PID to AdditionalData
+ additionalData.emplace("_PID", std::to_string(getpid()));
+
+ // Create FFDC files containing debug data to store in error log
+ std::vector<FFDCFile> files{createFFDCFiles(journal)};
+
+ // Create FFDC tuples used to pass FFDC files to D-Bus method
+ std::vector<FFDCTuple> ffdcTuples{createFFDCTuples(files)};
+
+ // Call D-Bus method to create an error log with FFDC files
+ const char* service = "xyz.openbmc_project.Logging";
+ const char* objPath = "/xyz/openbmc_project/logging";
+ const char* interface = "xyz.openbmc_project.Logging.Create";
+ const char* method = "CreateWithFFDCFiles";
+ auto reqMsg = bus.new_method_call(service, objPath, interface, method);
+ reqMsg.append(message, severity, additionalData, ffdcTuples);
+ auto respMsg = bus.call(reqMsg);
+
+ // Remove FFDC files. If an exception occurs before this, the files
+ // will be deleted by FFDCFile desctructor but errors will be ignored.
+ removeFFDCFiles(files, journal);
+ }
+ catch (const std::exception& e)
+ {
+ journal.logError(exception_utils::getMessages(e));
+ journal.logError("Unable to log error " + message);
+ }
+}
+
+void DBusErrorLogging::removeFFDCFiles(std::vector<FFDCFile>& files,
+ Journal& journal)
+{
+ // Explicitly remove FFDC files rather than relying on FFDCFile destructor.
+ // This allows any resulting errors to be written to the journal.
+ for (FFDCFile& file : files)
+ {
+ try
+ {
+ file.remove();
+ }
+ catch (const std::exception& e)
+ {
+ journal.logError(exception_utils::getMessages(e));
+ }
+ }
+
+ // Clear vector since the FFDCFile objects can no longer be used
+ files.clear();
+}
+
+} // namespace phosphor::power::regulators
diff --git a/phosphor-regulators/src/error_logging.hpp b/phosphor-regulators/src/error_logging.hpp
index ddd3b94..d1a5879 100644
--- a/phosphor-regulators/src/error_logging.hpp
+++ b/phosphor-regulators/src/error_logging.hpp
@@ -15,11 +15,26 @@
*/
#pragma once
+#include "ffdc_file.hpp"
+#include "journal.hpp"
+#include "xyz/openbmc_project/Logging/Create/server.hpp"
+#include "xyz/openbmc_project/Logging/Entry/server.hpp"
+
#include <sdbusplus/bus.hpp>
+#include <cstdint>
+#include <map>
+#include <string>
+#include <tuple>
+#include <vector>
+
namespace phosphor::power::regulators
{
+using namespace sdbusplus::xyz::openbmc_project::Logging::server;
+using FFDCTuple =
+ std::tuple<FFDCFormat, uint8_t, uint8_t, sdbusplus::message::unix_fd>;
+
/**
* @class ErrorLogging
*
@@ -38,13 +53,78 @@
ErrorLogging& operator=(ErrorLogging&&) = delete;
virtual ~ErrorLogging() = default;
- // TODO: The following are stubs. Add parameters and doxygen later.
- virtual void logConfigFileError() = 0;
- virtual void logDBusError() = 0;
- virtual void logI2CError() = 0;
- virtual void logInternalError() = 0;
- virtual void logPMBusError() = 0;
- virtual void logWriteVerificationError() = 0;
+ /**
+ * Log a regulators configuration file error.
+ *
+ * This error is logged when the regulators configuration file could not be
+ * found, could not be read, or had invalid contents.
+ *
+ * @param severity severity level
+ * @param journal system journal
+ */
+ virtual void logConfigFileError(Entry::Level severity,
+ Journal& journal) = 0;
+
+ /**
+ * Log a D-Bus error.
+ *
+ * This error is logged when D-Bus communication fails.
+ *
+ * @param severity severity level
+ * @param journal system journal
+ */
+ virtual void logDBusError(Entry::Level severity, Journal& journal) = 0;
+
+ /**
+ * Log an I2C communication error.
+ *
+ * @param severity severity level
+ * @param journal system journal
+ * @param bus I2C bus in the form "/dev/i2c-X", where X is the 0-based bus
+ * number
+ * @param addr 7 bit I2C address
+ * @param errorNumber errno value from the failed I2C operation
+ */
+ virtual void logI2CError(Entry::Level severity, Journal& journal,
+ const std::string& bus, uint8_t addr,
+ int errorNumber) = 0;
+
+ /**
+ * Log an internal firmware error.
+ *
+ * @param severity severity level
+ * @param journal system journal
+ */
+ virtual void logInternalError(Entry::Level severity, Journal& journal) = 0;
+
+ /**
+ * Log a PMBus error.
+ *
+ * This error is logged when the I2C communication was successful, but the
+ * PMBus value read is invalid or unsupported.
+ *
+ * @param severity severity level
+ * @param journal system journal
+ * @param inventoryPath D-Bus inventory path of the device where the error
+ * occurred
+ */
+ virtual void logPMBusError(Entry::Level severity, Journal& journal,
+ const std::string& inventoryPath) = 0;
+
+ /**
+ * Log a write verification error.
+ *
+ * This error is logged when a device register is written, read back, and
+ * the two values do not match. This is also called a read-back error.
+ *
+ * @param severity severity level
+ * @param journal system journal
+ * @param inventoryPath D-Bus inventory path of the device where the error
+ * occurred
+ */
+ virtual void
+ logWriteVerificationError(Entry::Level severity, Journal& journal,
+ const std::string& inventoryPath) = 0;
};
/**
@@ -72,17 +152,113 @@
{
}
- // TODO: The following are stubs. Add parameters, implementation, and
- // doxygen later.
- virtual void logConfigFileError(){};
- virtual void logDBusError(){};
- virtual void logI2CError(){};
- virtual void logInternalError(){};
- virtual void logPMBusError(){};
- virtual void logWriteVerificationError(){};
+ /** @copydoc ErrorLogging::logConfigFileError() */
+ virtual void logConfigFileError(Entry::Level severity,
+ Journal& journal) override;
+
+ /** @copydoc ErrorLogging::logDBusError() */
+ virtual void logDBusError(Entry::Level severity, Journal& journal) override;
+
+ /** @copydoc ErrorLogging::logI2CError() */
+ virtual void logI2CError(Entry::Level severity, Journal& journal,
+ const std::string& bus, uint8_t addr,
+ int errorNumber) override;
+
+ /** @copydoc ErrorLogging::logInternalError() */
+ virtual void logInternalError(Entry::Level severity,
+ Journal& journal) override;
+
+ /** @copydoc ErrorLogging::logPMBusError() */
+ virtual void logPMBusError(Entry::Level severity, Journal& journal,
+ const std::string& inventoryPath) override;
+
+ /** @copydoc ErrorLogging::logWriteVerificationError() */
+ virtual void
+ logWriteVerificationError(Entry::Level severity, Journal& journal,
+ const std::string& inventoryPath) override;
private:
/**
+ * Create an FFDCFile object containing the specified lines of text data.
+ *
+ * Throws an exception if an error occurs.
+ *
+ * @param lines lines of text data to write to file
+ * @return FFDCFile object
+ */
+ FFDCFile createFFDCFile(const std::vector<std::string>& lines);
+
+ /**
+ * Create FFDCFile objects containing debug data to store in the error log.
+ *
+ * If an error occurs, the error is written to the journal but an exception
+ * is not thrown.
+ *
+ * @param journal system journal
+ * @return vector of FFDCFile objects
+ */
+ std::vector<FFDCFile> createFFDCFiles(Journal& journal);
+
+ /**
+ * Create FFDCTuple objects corresponding to the specified FFDC files.
+ *
+ * The D-Bus method to create an error log requires a vector of tuples to
+ * pass in the FFDC file information.
+ *
+ * @param files FFDC files
+ * @return vector of FFDCTuple objects
+ */
+ std::vector<FFDCTuple> createFFDCTuples(std::vector<FFDCFile>& files);
+
+ /**
+ * Returns the absolute form of the specified inventory path.
+ *
+ * The inventory paths in the JSON configuration file are relative. Add the
+ * the necessary prefix to make the path absolute.
+ *
+ * @param inventoryPath relative D-Bus inventory path
+ * @return absolute D-Bus inventory path
+ */
+ std::string getAbsoluteInventoryPath(const std::string& inventoryPath)
+ {
+ std::string absPath = "/xyz/openbmc_project/inventory";
+ if ((!inventoryPath.empty()) && (inventoryPath.front() != '/'))
+ {
+ absPath += '/';
+ }
+ absPath += inventoryPath;
+ return absPath;
+ }
+
+ /**
+ * Logs an error using the D-Bus CreateWithFFDCFiles method.
+ *
+ * If logging fails, a message is written to the journal but an exception is
+ * not thrown.
+ *
+ * @param message Message property of the error log entry
+ * @param severity Severity property of the error log entry
+ * @param additionalData AdditionalData property of the error log entry
+ * @param journal system journal
+ */
+ void logError(const std::string& message, Entry::Level severity,
+ std::map<std::string, std::string>& additionalData,
+ Journal& journal);
+
+ /**
+ * Removes the specified FFDC files from the file system.
+ *
+ * Also clears the specified vector, removing the FFDCFile objects.
+ *
+ * If an error occurs, the error is written to the journal but an exception
+ * is not thrown.
+ *
+ * @param files FFDC files to remove
+ * @param journal system journal
+ */
+ void removeFFDCFiles(std::vector<FFDCFile>& files, Journal& journal);
+
+ /**
* D-Bus bus object.
*/
sdbusplus::bus::bus& bus;
diff --git a/phosphor-regulators/src/meson.build b/phosphor-regulators/src/meson.build
index 8303769..b6b500c 100644
--- a/phosphor-regulators/src/meson.build
+++ b/phosphor-regulators/src/meson.build
@@ -9,6 +9,7 @@
'config_file_parser.cpp',
'configuration.cpp',
'device.cpp',
+ 'error_logging.cpp',
'exception_utils.cpp',
'ffdc_file.cpp',
'id_map.cpp',
diff --git a/phosphor-regulators/test/mock_error_logging.hpp b/phosphor-regulators/test/mock_error_logging.hpp
index 697af3e..db75a37 100644
--- a/phosphor-regulators/test/mock_error_logging.hpp
+++ b/phosphor-regulators/test/mock_error_logging.hpp
@@ -38,13 +38,29 @@
MockErrorLogging& operator=(MockErrorLogging&&) = delete;
virtual ~MockErrorLogging() = default;
- // TODO: Add parameters when ErrorLogging interface is complete
- MOCK_METHOD(void, logConfigFileError, (), (override));
- MOCK_METHOD(void, logDBusError, (), (override));
- MOCK_METHOD(void, logI2CError, (), (override));
- MOCK_METHOD(void, logInternalError, (), (override));
- MOCK_METHOD(void, logPMBusError, (), (override));
- MOCK_METHOD(void, logWriteVerificationError, (), (override));
+ MOCK_METHOD(void, logConfigFileError,
+ (Entry::Level severity, Journal& journal), (override));
+
+ MOCK_METHOD(void, logDBusError, (Entry::Level severity, Journal& journal),
+ (override));
+
+ MOCK_METHOD(void, logI2CError,
+ (Entry::Level severity, Journal& journal,
+ const std::string& bus, uint8_t addr, int errorNumber),
+ (override));
+
+ MOCK_METHOD(void, logInternalError,
+ (Entry::Level severity, Journal& journal), (override));
+
+ MOCK_METHOD(void, logPMBusError,
+ (Entry::Level severity, Journal& journal,
+ const std::string& inventoryPath),
+ (override));
+
+ MOCK_METHOD(void, logWriteVerificationError,
+ (Entry::Level severity, Journal& journal,
+ const std::string& inventoryPath),
+ (override));
};
} // namespace phosphor::power::regulators