diff options
| -rw-r--r-- | occ_device.cpp | 23 | ||||
| -rw-r--r-- | occ_device.hpp | 60 | ||||
| -rw-r--r-- | occ_errors.cpp | 9 | ||||
| -rw-r--r-- | occ_errors.hpp | 19 | ||||
| -rw-r--r-- | occ_status.hpp | 5 | ||||
| -rw-r--r-- | test/Makefile.am | 3 | ||||
| -rw-r--r-- | test/error_files_tests.cpp | 103 | ||||
| -rw-r--r-- | test/utest.cpp | 25 |
8 files changed, 218 insertions, 29 deletions
diff --git a/occ_device.cpp b/occ_device.cpp index b05dc09..6cd6a1f 100644 --- a/occ_device.cpp +++ b/occ_device.cpp @@ -12,10 +12,31 @@ namespace occ fs::path Device::bindPath = fs::path(OCC_HWMON_PATH) / "bind"; fs::path Device::unBindPath = fs::path(OCC_HWMON_PATH) / "unbind"; +std::string Device::getPathBack(const fs::path& path) +{ + if (path.empty()) + return std::string(); + + // Points to the last element in the path + auto conf = --path.end(); + + // The last element will be '.' if the path ends in '/' + // This behavior differs between filesystem and experimental::filesystem + // Verify there is an element before too + if (!conf->compare(".") && conf != path.begin()) + { + return *(--conf); + } + else + { + return *conf; + } +} + bool Device::master() const { int master; - auto masterFile = fs::path(DEV_PATH) / config / "occ_master"; + auto masterFile = devPath / "occ_master"; std::ifstream file(masterFile, std::ios::in); if (!file) diff --git a/occ_device.hpp b/occ_device.hpp index 448253a..937c6dc 100644 --- a/occ_device.hpp +++ b/occ_device.hpp @@ -8,6 +8,7 @@ #include <experimental/filesystem> #include <fstream> +#include <org/open_power/OCC/Device/error.hpp> namespace open_power { @@ -17,6 +18,7 @@ namespace occ class Manager; class Status; namespace fs = std::experimental::filesystem; +using namespace sdbusplus::org::open_power::OCC::Device::Error; /** @class Device * @brief Binds and unbinds the OCC driver upon request @@ -34,25 +36,25 @@ class Device /** @brief Constructs the Device object * * @param[in] event - Unique ptr reference to sd_event - * @param[in] name - OCC instance name + * @param[in] path - Path to the OCC instance * @param[in] manager - OCC manager instance * @param[in] callback - Optional callback on errors */ - Device(EventPtr& event, const std::string& name, const Manager& manager, + Device(EventPtr& event, const fs::path& path, const Manager& manager, Status& status, std::function<void(bool)> callBack = nullptr) : - config(name), - errorFile(fs::path(config) / "occ_error"), statusObject(status), - error(event, errorFile, callBack), - presence(event, fs::path(config) / "occs_present", manager, callBack), + config(getPathBack(path)), + devPath(path), statusObject(status), + error(event, path / "occ_error", callBack), + presence(event, path / "occs_present", manager, callBack), throttleProcTemp( - event, fs::path(config) / "occ_dvfs_ot", + event, path / "occ_dvfs_overtemp", std::bind(std::mem_fn(&Device::throttleProcTempCallback), this, std::placeholders::_1)), throttleProcPower( - event, fs::path(config) / "occ_dvfs_power", + event, path / "occ_dvfs_power", std::bind(std::mem_fn(&Device::throttleProcPowerCallback), this, std::placeholders::_1)), - throttleMemTemp(event, fs::path(config) / "occ_mem_throttle", + throttleMemTemp(event, path / "occ_mem_throttle", std::bind(std::mem_fn(&Device::throttleMemTempCallback), this, std::placeholders::_1)) { @@ -86,13 +88,29 @@ class Device return fs::exists(OCC_HWMON_PATH + config); } - /** @brief Starts to monitor for errors */ - inline void addErrorWatch() + /** @brief Starts to monitor for errors + * + * @param[in] poll - Indicates whether or not the error file should + * actually be polled for changes. Disabling polling is + * necessary for error files that don't support the poll + * file operation. + */ + inline void addErrorWatch(bool poll = true) { - throttleProcTemp.addWatch(); - throttleProcPower.addWatch(); - throttleMemTemp.addWatch(); - error.addWatch(); + try + { + throttleProcTemp.addWatch(poll); + } + catch (const OpenFailure& e) + { + // try the old kernel version + throttleProcTemp.setFile(devPath / "occ_dvfs_ot"); + throttleProcTemp.addWatch(poll); + } + + throttleProcPower.addWatch(poll); + throttleMemTemp.addWatch(poll); + error.addWatch(poll); } /** @brief stops monitoring for errors */ @@ -101,7 +119,6 @@ class Device // we can always safely remove watch even if we don't add it presence.removeWatch(); error.removeWatch(); - error.removeWatch(); throttleMemTemp.removeWatch(); throttleProcPower.removeWatch(); throttleProcTemp.removeWatch(); @@ -116,12 +133,19 @@ class Device } } + /** @brief helper function to get the last part of the path + * + * @param[in] path - Path to parse + * @return - Last directory name in the path + */ + static std::string getPathBack(const fs::path& path); + private: /** @brief Config value to be used to do bind and unbind */ const std::string config; - /** @brief This file contains 0 for success, non-zero for errors */ - const fs::path errorFile; + /** @brief This directory contains the error files */ + const fs::path devPath; /** @brief To bind the device to the OCC driver, do: * diff --git a/occ_errors.cpp b/occ_errors.cpp index ae66a62..ab7663a 100644 --- a/occ_errors.cpp +++ b/occ_errors.cpp @@ -56,15 +56,18 @@ void Error::registerCallBack() } // Starts to watch for errors -void Error::addWatch() +void Error::addWatch(bool poll) { if (!watching) { // Open the file openFile(); - // register the callback handler - registerCallBack(); + if (poll) + { + // register the callback handler + registerCallBack(); + } // Set we are watching the error watching = true; diff --git a/occ_errors.hpp b/occ_errors.hpp index ebacc8a..f623704 100644 --- a/occ_errors.hpp +++ b/occ_errors.hpp @@ -36,7 +36,7 @@ class Error Error(EventPtr& event, const fs::path& file, std::function<void(bool)> callBack = nullptr) : event(event), - file(fs::path(DEV_PATH) / file), callBack(callBack) + file(file), callBack(callBack) { // Nothing to do here. } @@ -49,12 +49,23 @@ class Error } } - /** @brief Starts to monitor for error conditions */ - void addWatch(); + /** @brief Starts to monitor for error conditions + * + * @param[in] poll - Indicates whether or not the error file should + * actually be polled for changes. Disabling polling is + * necessary for error files that don't support the poll + * file operation. + */ + void addWatch(bool poll = true); /** @brief Removes error watch */ void removeWatch(); + inline void setFile(const fs::path& f) + { + file = f; + } + private: /** @brief sd_event wrapped in unique_ptr */ EventPtr& event; @@ -95,7 +106,7 @@ class Error int fd = -1; /** Error file */ - const fs::path file; + fs::path file; /** @brief Optional function to call on error scenario */ std::function<void(bool)> callBack; diff --git a/occ_status.hpp b/occ_status.hpp index 9e6c687..859b5b0 100644 --- a/occ_status.hpp +++ b/occ_status.hpp @@ -65,9 +65,10 @@ class Status : public Interface instance(((this->path.back() - '0'))), device(event, #ifdef I2C_OCC - i2c_occ::getI2cDeviceName(path), + fs::path(DEV_PATH) / i2c_occ::getI2cDeviceName(path), #else - sysfsName + "." + std::to_string(instance + 1), + fs::path(DEV_PATH) / + fs::path(sysfsName + "." + std::to_string(instance + 1)), #endif manager, *this, std::bind(std::mem_fn(&Status::deviceErrorHandler), this, diff --git a/test/Makefile.am b/test/Makefile.am index ff56179..0ded744 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -6,7 +6,8 @@ TESTS = $(check_PROGRAMS) utest_LDADD = $(top_builddir)/libocc_control.la -lstdc++fs -utest_SOURCES = utest.cpp \ +utest_SOURCES = error_files_tests.cpp \ + utest.cpp \ TestI2cOcc.cpp utest_CPPFLAGS = $(GTEST_CPPFLAGS) \ diff --git a/test/error_files_tests.cpp b/test/error_files_tests.cpp new file mode 100644 index 0000000..532818a --- /dev/null +++ b/test/error_files_tests.cpp @@ -0,0 +1,103 @@ +#include "occ_manager.hpp" + +#include <stdlib.h> + +#include <filesystem> +#include <fstream> + +#include <gtest/gtest.h> + +constexpr auto num_error_files = 8; +constexpr auto device = "occ-hwmon.1"; +constexpr auto error = "occ_error"; +constexpr auto errorMem = "occ_mem_throttle"; +constexpr auto errorPower = "occ_dvfs_power"; +constexpr auto errorTemp = "occ_dvfs_overtemp"; +constexpr auto legacyDevice = "occ-hwmon.2"; +constexpr auto legacyErrorTemp = "occ_dvfs_ot"; +constexpr auto noError = "0"; + +namespace fs = std::experimental::filesystem; +using namespace open_power::occ; + +class ErrorFiles : public ::testing::Test +{ + public: + ErrorFiles() : + bus(sdbusplus::bus::new_default()), rc(sd_event_default(&event)), + pEvent(event), manager(bus, pEvent), + status(bus, pEvent, "/dummy1", manager) + { + EXPECT_GE(rc, 0); + event = nullptr; + } + + virtual void SetUp() + { + fs::path files[num_error_files]; + char tmpDirTemplate[64]; + + strcpy(tmpDirTemplate, "/tmp/occXXXXXX"); + auto path = mkdtemp(tmpDirTemplate); + assert(path != nullptr); + + occPath = path; + devicePath = occPath / device; + legacyDevicePath = occPath / legacyDevice; + + fs::create_directory(devicePath); + fs::create_directory(legacyDevicePath); + + files[0] = devicePath / error; + files[1] = devicePath / errorMem; + files[2] = devicePath / errorPower; + files[3] = devicePath / errorTemp; + files[4] = legacyDevicePath / error; + files[5] = legacyDevicePath / errorMem; + files[6] = legacyDevicePath / errorPower; + files[7] = legacyDevicePath / legacyErrorTemp; + + for (const fs::path& f : files) + { + auto stream = std::ofstream(f.c_str()); + + if (stream) + { + stream << noError; + } + } + } + + virtual void TearDown() + { + fs::remove_all(occPath); + } + + sdbusplus::bus::bus bus; + sd_event* event; + int rc; + open_power::occ::EventPtr pEvent; + + Manager manager; + Status status; + + fs::path devicePath; + fs::path legacyDevicePath; + fs::path occPath; +}; + +TEST_F(ErrorFiles, AddDeviceErrorWatch) +{ + Device occDevice(pEvent, devicePath, manager, status); + + occDevice.addErrorWatch(false); + occDevice.removeErrorWatch(); +} + +TEST_F(ErrorFiles, AddLegacyDeviceErrorWatch) +{ + Device legacyOccDevice(pEvent, legacyDevicePath, manager, status); + + legacyOccDevice.addErrorWatch(false); + legacyOccDevice.removeErrorWatch(); +} diff --git a/test/utest.cpp b/test/utest.cpp index eb9ca0b..39e5250 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -1,5 +1,6 @@ #include "powercap.hpp" +#include <experimental/filesystem> #include <occ_events.hpp> #include <occ_manager.hpp> @@ -43,3 +44,27 @@ TEST_F(VerifyOccInput, PcapEnabled) uint32_t occInput = pcap.getOccInput(100, true); EXPECT_EQ(occInput, 90); } + +TEST(VerifyPathParsing, EmptyPath) +{ + std::experimental::filesystem::path path = ""; + std::string parsed = Device::getPathBack(path); + + EXPECT_STREQ(parsed.c_str(), ""); +} + +TEST(VerifyPathParsing, FilenamePath) +{ + std::experimental::filesystem::path path = "/test/foo.bar"; + std::string parsed = Device::getPathBack(path); + + EXPECT_STREQ(parsed.c_str(), "foo.bar"); +} + +TEST(VerifyPathParsing, DirectoryPath) +{ + std::experimental::filesystem::path path = "/test/bar/"; + std::string parsed = Device::getPathBack(path); + + EXPECT_STREQ(parsed.c_str(), "bar"); +} |
